From: Paul Eggert Date: Fri, 19 Jul 2013 05:36:50 +0000 (-0700) Subject: * sysdep.c [GNU_LINUX]: Fix fd and memory leaks and similar issues. X-Git-Tag: emacs-24.3.90~173^2^2~42^2~45^2~387^2~1759^2~14 X-Git-Url: http://git.eshelyaron.com/gitweb/?a=commitdiff_plain;h=ab9980cd3ba7e1cefe846610dccdc86923dcdb86;p=emacs.git * sysdep.c [GNU_LINUX]: Fix fd and memory leaks and similar issues. (procfs_ttyname): Don't use uninitialized storage if emacs_fopen or fscanf fails. (system_process_attributes): Prefer plain char to unsigned char when either will do. Clean up properly if interrupted or if memory allocations fail. Don't assume sscanf succeeds. Remove no-longer-needed workaround to stop GCC from whining. Read command-line once, instead of multiple times. Check read status a bit more carefully. --- diff --git a/src/ChangeLog b/src/ChangeLog index 73fc64f37c5..b0c486ab8b2 100644 --- a/src/ChangeLog +++ b/src/ChangeLog @@ -1,5 +1,15 @@ 2013-07-19 Paul Eggert + * sysdep.c [GNU_LINUX]: Fix fd and memory leaks and similar issues. + (procfs_ttyname): Don't use uninitialized storage if emacs_fopen + or fscanf fails. + (system_process_attributes): Prefer plain char to unsigned char + when either will do. Clean up properly if interrupted or if + memory allocations fail. Don't assume sscanf succeeds. Remove + no-longer-needed workaround to stop GCC from whining. Read + command-line once, instead of multiple times. Check read status a + bit more carefully. + Fix obscure porting bug with varargs functions. The code assumed that int is treated like ptrdiff_t in a vararg function, which is not a portable assumption. There was a similar diff --git a/src/sysdep.c b/src/sysdep.c index 465d271abca..2739583456a 100644 --- a/src/sysdep.c +++ b/src/sysdep.c @@ -2807,11 +2807,12 @@ get_up_time (void) static Lisp_Object procfs_ttyname (int rdev) { - FILE *fdev = NULL; + FILE *fdev; char name[PATH_MAX]; block_input (); fdev = emacs_fopen ("/proc/tty/drivers", "r"); + name[0] = 0; if (fdev) { @@ -2820,7 +2821,7 @@ procfs_ttyname (int rdev) char minor[25]; /* 2 32-bit numbers + dash */ char *endp; - while (!feof (fdev) && !ferror (fdev)) + for (; !feof (fdev) && !ferror (fdev); name[0] = 0) { if (fscanf (fdev, "%*s %s %u %s %*s\n", name, &major, minor) >= 3 && major == MAJOR (rdev)) @@ -2849,7 +2850,7 @@ procfs_ttyname (int rdev) static unsigned long procfs_get_total_memory (void) { - FILE *fmem = NULL; + FILE *fmem; unsigned long retval = 2 * 1024 * 1024; /* default: 2GB */ block_input (); @@ -2892,7 +2893,7 @@ system_process_attributes (Lisp_Object pid) int cmdsize = sizeof default_cmd - 1; char *cmdline = NULL; ptrdiff_t cmdline_size; - unsigned char c; + char c; printmax_t proc_id; int ppid, pgrp, sess, tty, tpgid, thcount; uid_t uid; @@ -2903,7 +2904,8 @@ system_process_attributes (Lisp_Object pid) EMACS_TIME tnow, tstart, tboot, telapsed, us_time; double pcpu, pmem; Lisp_Object attrs = Qnil; - Lisp_Object cmd_str, decoded_cmd, tem; + Lisp_Object cmd_str, decoded_cmd; + ptrdiff_t count; struct gcpro gcpro1, gcpro2; CHECK_NUMBER_OR_FLOAT (pid); @@ -2931,11 +2933,19 @@ system_process_attributes (Lisp_Object pid) if (gr) attrs = Fcons (Fcons (Qgroup, build_string (gr->gr_name)), attrs); + count = SPECPDL_INDEX (); strcpy (fn, procfn); procfn_end = fn + strlen (fn); strcpy (procfn_end, "/stat"); fd = emacs_open (fn, O_RDONLY, 0); - if (fd >= 0 && (nread = emacs_read (fd, procbuf, sizeof (procbuf) - 1)) > 0) + if (fd < 0) + nread = 0; + else + { + record_unwind_protect_int (close_file_unwind, fd); + nread = emacs_read (fd, procbuf, sizeof procbuf - 1); + } + if (0 < nread) { procbuf[nread] = '\0'; p = procbuf; @@ -2959,39 +2969,32 @@ system_process_attributes (Lisp_Object pid) Vlocale_coding_system, 0); attrs = Fcons (Fcons (Qcomm, decoded_cmd), attrs); - if (q) + /* state ppid pgrp sess tty tpgid . minflt cminflt majflt cmajflt + utime stime cutime cstime priority nice thcount . start vsize rss */ + if (q + && (sscanf (q + 2, ("%c %d %d %d %d %d %*u %lu %lu %lu %lu " + "%Lu %Lu %Lu %Lu %ld %ld %d %*d %Lu %lu %ld"), + &c, &ppid, &pgrp, &sess, &tty, &tpgid, + &minflt, &cminflt, &majflt, &cmajflt, + &u_time, &s_time, &cutime, &cstime, + &priority, &niceness, &thcount, &start, &vsize, &rss) + == 20)) { - EMACS_INT ppid_eint, pgrp_eint, sess_eint, tpgid_eint, thcount_eint; - p = q + 2; - /* state ppid pgrp sess tty tpgid . minflt cminflt majflt cmajflt utime stime cutime cstime priority nice thcount . start vsize rss */ - sscanf (p, "%c %d %d %d %d %d %*u %lu %lu %lu %lu %Lu %Lu %Lu %Lu %ld %ld %d %*d %Lu %lu %ld", - &c, &ppid, &pgrp, &sess, &tty, &tpgid, - &minflt, &cminflt, &majflt, &cmajflt, - &u_time, &s_time, &cutime, &cstime, - &priority, &niceness, &thcount, &start, &vsize, &rss); - { - char state_str[2]; - - state_str[0] = c; - state_str[1] = '\0'; - tem = build_string (state_str); - attrs = Fcons (Fcons (Qstate, tem), attrs); - } - /* Stops GCC whining about limited range of data type. */ - ppid_eint = ppid; - pgrp_eint = pgrp; - sess_eint = sess; - tpgid_eint = tpgid; - thcount_eint = thcount; - attrs = Fcons (Fcons (Qppid, make_fixnum_or_float (ppid_eint)), attrs); - attrs = Fcons (Fcons (Qpgrp, make_fixnum_or_float (pgrp_eint)), attrs); - attrs = Fcons (Fcons (Qsess, make_fixnum_or_float (sess_eint)), attrs); + char state_str[2]; + state_str[0] = c; + state_str[1] = '\0'; + attrs = Fcons (Fcons (Qstate, build_string (state_str)), attrs); + attrs = Fcons (Fcons (Qppid, make_fixnum_or_float (ppid)), attrs); + attrs = Fcons (Fcons (Qpgrp, make_fixnum_or_float (pgrp)), attrs); + attrs = Fcons (Fcons (Qsess, make_fixnum_or_float (sess)), attrs); attrs = Fcons (Fcons (Qttname, procfs_ttyname (tty)), attrs); - attrs = Fcons (Fcons (Qtpgid, make_fixnum_or_float (tpgid_eint)), attrs); + attrs = Fcons (Fcons (Qtpgid, make_fixnum_or_float (tpgid)), attrs); attrs = Fcons (Fcons (Qminflt, make_fixnum_or_float (minflt)), attrs); attrs = Fcons (Fcons (Qmajflt, make_fixnum_or_float (majflt)), attrs); - attrs = Fcons (Fcons (Qcminflt, make_fixnum_or_float (cminflt)), attrs); - attrs = Fcons (Fcons (Qcmajflt, make_fixnum_or_float (cmajflt)), attrs); + attrs = Fcons (Fcons (Qcminflt, make_fixnum_or_float (cminflt)), + attrs); + attrs = Fcons (Fcons (Qcmajflt, make_fixnum_or_float (cmajflt)), + attrs); clocks_per_sec = sysconf (_SC_CLK_TCK); if (clocks_per_sec < 0) clocks_per_sec = 100; @@ -3012,19 +3015,22 @@ system_process_attributes (Lisp_Object pid) ltime_from_jiffies (cstime, clocks_per_sec)), attrs); attrs = Fcons (Fcons (Qctime, - ltime_from_jiffies (cstime+cutime, clocks_per_sec)), + ltime_from_jiffies (cstime + cutime, + clocks_per_sec)), attrs); attrs = Fcons (Fcons (Qpri, make_number (priority)), attrs); attrs = Fcons (Fcons (Qnice, make_number (niceness)), attrs); - attrs = Fcons (Fcons (Qthcount, make_fixnum_or_float (thcount_eint)), attrs); + attrs = Fcons (Fcons (Qthcount, make_fixnum_or_float (thcount)), + attrs); tnow = current_emacs_time (); telapsed = get_up_time (); tboot = sub_emacs_time (tnow, telapsed); tstart = time_from_jiffies (start, clocks_per_sec); tstart = add_emacs_time (tboot, tstart); attrs = Fcons (Fcons (Qstart, make_lisp_time (tstart)), attrs); - attrs = Fcons (Fcons (Qvsize, make_fixnum_or_float (vsize/1024)), attrs); - attrs = Fcons (Fcons (Qrss, make_fixnum_or_float (4*rss)), attrs); + attrs = Fcons (Fcons (Qvsize, make_fixnum_or_float (vsize / 1024)), + attrs); + attrs = Fcons (Fcons (Qrss, make_fixnum_or_float (4 * rss)), attrs); telapsed = sub_emacs_time (tnow, tstart); attrs = Fcons (Fcons (Qetime, make_lisp_time (telapsed)), attrs); us_time = time_from_jiffies (u_time + s_time, clocks_per_sec); @@ -3039,67 +3045,63 @@ system_process_attributes (Lisp_Object pid) attrs = Fcons (Fcons (Qpmem, make_float (pmem)), attrs); } } - if (fd >= 0) - emacs_close (fd); + unbind_to (count, Qnil); /* args */ strcpy (procfn_end, "/cmdline"); fd = emacs_open (fn, O_RDONLY, 0); if (fd >= 0) { - char ch; - for (cmdline_size = 0; cmdline_size < STRING_BYTES_BOUND; cmdline_size++) + ptrdiff_t readsize, nread_incr; + record_unwind_protect_int (close_file_unwind, fd); + record_unwind_protect_nothing (); + nread = cmdline_size = 0; + + do { - if (emacs_read (fd, &ch, 1) != 1) - break; - c = ch; - if (c_isspace (c) || c == '\\') - cmdline_size++; /* for later quoting, see below */ + cmdline = xpalloc (cmdline, &cmdline_size, 2, STRING_BYTES_BOUND, 1); + set_unwind_protect_ptr (count + 1, xfree, cmdline); + + /* Leave room even if every byte needs escaping below. */ + readsize = (cmdline_size >> 1) - nread; + + nread_incr = emacs_read (fd, cmdline + nread, readsize); + nread += max (0, nread_incr); } - if (cmdline_size) + while (nread_incr == readsize); + + if (nread) { - cmdline = xmalloc (cmdline_size + 1); - lseek (fd, 0L, SEEK_SET); - cmdline[0] = '\0'; - if ((nread = read (fd, cmdline, cmdline_size)) >= 0) - cmdline[nread++] = '\0'; - else - { - /* Assigning zero to `nread' makes us skip the following - two loops, assign zero to cmdline_size, and enter the - following `if' clause that handles unknown command - lines. */ - nread = 0; - } /* We don't want trailing null characters. */ - for (p = cmdline + nread; p > cmdline + 1 && !p[-1]; p--) - nread--; - for (p = cmdline; p < cmdline + nread; p++) + for (p = cmdline + nread; cmdline < p && !p[-1]; p--) + continue; + + /* Escape-quote whitespace and backslashes. */ + q = cmdline + cmdline_size; + while (cmdline < p) { - /* Escape-quote whitespace and backslashes. */ - if (c_isspace (*p) || *p == '\\') - { - memmove (p + 1, p, nread - (p - cmdline)); - nread++; - *p++ = '\\'; - } - else if (*p == '\0') - *p = ' '; + char c = *--p; + *--q = c ? c : ' '; + if (c_isspace (c) || c == '\\') + *--q = '\\'; } - cmdline_size = nread; + + nread = cmdline + cmdline_size - q; } - if (!cmdline_size) + + if (!nread) { - cmdline_size = cmdsize + 2; - cmdline = xmalloc (cmdline_size + 1); + nread = cmdsize + 2; + cmdline_size = nread + 1; + q = cmdline = xrealloc (cmdline, cmdline_size); + set_unwind_protect_ptr (count + 1, xfree, cmdline); sprintf (cmdline, "[%.*s]", cmdsize, cmd); } - emacs_close (fd); /* Command line is encoded in locale-coding-system; decode it. */ - cmd_str = make_unibyte_string (cmdline, cmdline_size); + cmd_str = make_unibyte_string (q, nread); decoded_cmd = code_convert_string_norecord (cmd_str, Vlocale_coding_system, 0); - xfree (cmdline); + unbind_to (count, Qnil); attrs = Fcons (Fcons (Qargs, decoded_cmd), attrs); } @@ -3141,8 +3143,9 @@ system_process_attributes (Lisp_Object pid) uid_t uid; gid_t gid; Lisp_Object attrs = Qnil; - Lisp_Object decoded_cmd, tem; + Lisp_Object decoded_cmd; struct gcpro gcpro1, gcpro2; + ptrdiff_t count; CHECK_NUMBER_OR_FLOAT (pid); CONS_TO_INTEGER (pid, pid_t, proc_id); @@ -3169,72 +3172,83 @@ system_process_attributes (Lisp_Object pid) if (gr) attrs = Fcons (Fcons (Qgroup, build_string (gr->gr_name)), attrs); + count = SPECPDL_INDEX (); strcpy (fn, procfn); procfn_end = fn + strlen (fn); strcpy (procfn_end, "/psinfo"); fd = emacs_open (fn, O_RDONLY, 0); - if (fd >= 0 - && (nread = read (fd, (char*)&pinfo, sizeof (struct psinfo)) > 0)) + if (fd < 0) + nread = 0; + else { - attrs = Fcons (Fcons (Qppid, make_fixnum_or_float (pinfo.pr_ppid)), attrs); - attrs = Fcons (Fcons (Qpgrp, make_fixnum_or_float (pinfo.pr_pgid)), attrs); - attrs = Fcons (Fcons (Qsess, make_fixnum_or_float (pinfo.pr_sid)), attrs); - - { - char state_str[2]; - state_str[0] = pinfo.pr_lwp.pr_sname; - state_str[1] = '\0'; - tem = build_string (state_str); - attrs = Fcons (Fcons (Qstate, tem), attrs); - } - - /* FIXME: missing Qttyname. psinfo.pr_ttydev is a dev_t, - need to get a string from it. */ - - /* FIXME: missing: Qtpgid */ - - /* FIXME: missing: - Qminflt - Qmajflt - Qcminflt - Qcmajflt - - Qutime - Qcutime - Qstime - Qcstime - Are they available? */ - - attrs = Fcons (Fcons (Qtime, make_lisp_time (pinfo.pr_time)), attrs); - attrs = Fcons (Fcons (Qctime, make_lisp_time (pinfo.pr_ctime)), attrs); - attrs = Fcons (Fcons (Qpri, make_number (pinfo.pr_lwp.pr_pri)), attrs); - attrs = Fcons (Fcons (Qnice, make_number (pinfo.pr_lwp.pr_nice)), attrs); - attrs = Fcons (Fcons (Qthcount, make_fixnum_or_float (pinfo.pr_nlwp)), attrs); - - attrs = Fcons (Fcons (Qstart, make_lisp_time (pinfo.pr_start)), attrs); - attrs = Fcons (Fcons (Qvsize, make_fixnum_or_float (pinfo.pr_size)), attrs); - attrs = Fcons (Fcons (Qrss, make_fixnum_or_float (pinfo.pr_rssize)), attrs); - - /* pr_pctcpu and pr_pctmem are unsigned integers in the - range 0 .. 2**15, representing 0.0 .. 1.0. */ - attrs = Fcons (Fcons (Qpcpu, make_float (100.0 / 0x8000 * pinfo.pr_pctcpu)), attrs); - attrs = Fcons (Fcons (Qpmem, make_float (100.0 / 0x8000 * pinfo.pr_pctmem)), attrs); - - decoded_cmd - = code_convert_string_norecord (make_unibyte_string (pinfo.pr_fname, - strlen (pinfo.pr_fname)), - Vlocale_coding_system, 0); - attrs = Fcons (Fcons (Qcomm, decoded_cmd), attrs); - decoded_cmd - = code_convert_string_norecord (make_unibyte_string (pinfo.pr_psargs, - strlen (pinfo.pr_psargs)), - Vlocale_coding_system, 0); - attrs = Fcons (Fcons (Qargs, decoded_cmd), attrs); + record_unwind_protect (close_file_unwind, fd); + nread = emacs_read (fd, &pinfo, sizeof pinfo); } - if (fd >= 0) - emacs_close (fd); + if (nread == sizeof pinfo) + { + attrs = Fcons (Fcons (Qppid, make_fixnum_or_float (pinfo.pr_ppid)), attrs); + attrs = Fcons (Fcons (Qpgrp, make_fixnum_or_float (pinfo.pr_pgid)), attrs); + attrs = Fcons (Fcons (Qsess, make_fixnum_or_float (pinfo.pr_sid)), attrs); + + { + char state_str[2]; + state_str[0] = pinfo.pr_lwp.pr_sname; + state_str[1] = '\0'; + attrs = Fcons (Fcons (Qstate, build_string (state_str)), attrs); + } + /* FIXME: missing Qttyname. psinfo.pr_ttydev is a dev_t, + need to get a string from it. */ + + /* FIXME: missing: Qtpgid */ + + /* FIXME: missing: + Qminflt + Qmajflt + Qcminflt + Qcmajflt + + Qutime + Qcutime + Qstime + Qcstime + Are they available? */ + + attrs = Fcons (Fcons (Qtime, make_lisp_time (pinfo.pr_time)), attrs); + attrs = Fcons (Fcons (Qctime, make_lisp_time (pinfo.pr_ctime)), attrs); + attrs = Fcons (Fcons (Qpri, make_number (pinfo.pr_lwp.pr_pri)), attrs); + attrs = Fcons (Fcons (Qnice, make_number (pinfo.pr_lwp.pr_nice)), attrs); + attrs = Fcons (Fcons (Qthcount, make_fixnum_or_float (pinfo.pr_nlwp)), + attrs); + + attrs = Fcons (Fcons (Qstart, make_lisp_time (pinfo.pr_start)), attrs); + attrs = Fcons (Fcons (Qvsize, make_fixnum_or_float (pinfo.pr_size)), + attrs); + attrs = Fcons (Fcons (Qrss, make_fixnum_or_float (pinfo.pr_rssize)), + attrs); + + /* pr_pctcpu and pr_pctmem are unsigned integers in the + range 0 .. 2**15, representing 0.0 .. 1.0. */ + attrs = Fcons (Fcons (Qpcpu, + make_float (100.0 / 0x8000 * pinfo.pr_pctcpu)), + attrs); + attrs = Fcons (Fcons (Qpmem, + make_float (100.0 / 0x8000 * pinfo.pr_pctmem)), + attrs); + + decoded_cmd = (code_convert_string_norecord + (make_unibyte_string (pinfo.pr_fname, + strlen (pinfo.pr_fname)), + Vlocale_coding_system, 0)); + attrs = Fcons (Fcons (Qcomm, decoded_cmd), attrs); + decoded_cmd = (code_convert_string_norecord + (make_unibyte_string (pinfo.pr_psargs, + strlen (pinfo.pr_psargs)), + Vlocale_coding_system, 0)); + attrs = Fcons (Fcons (Qargs, decoded_cmd), attrs); + } + unbind_to (count, Qnil); UNGCPRO; return attrs; }