From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 29754 invoked by alias); 31 Dec 2012 19:41:54 -0000 Received: (qmail 29737 invoked by uid 22791); 31 Dec 2012 19:41:53 -0000 X-SWARE-Spam-Status: No, hits=-6.1 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,RCVD_IN_DNSWL_HI,SPF_HELO_PASS,TW_CP,TW_XS,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 31 Dec 2012 19:41:42 +0000 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id qBVJff6h028976 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Mon, 31 Dec 2012 14:41:41 -0500 Received: from host2.jankratochvil.net (ovpn-116-24.ams2.redhat.com [10.36.116.24]) by int-mx02.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id qBVJfZxI020470 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NO); Mon, 31 Dec 2012 14:41:38 -0500 Date: Mon, 31 Dec 2012 19:41:00 -0000 From: Jan Kratochvil To: Sergio Durigan Junior Cc: GDB Patches , Binutils Development , Pedro Alves , "H.J. Lu" Subject: Re: [PATCH/RFC 02/02 v2] Refactor PRPSINFO handling on GDB Message-ID: <20121231194134.GA17955@host2.jankratochvil.net> References: <20121218171555.GA19639@host2.jankratochvil.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2012-12/txt/msg00842.txt.bz2 On Sun, 30 Dec 2012 02:53:18 +0100, Sergio Durigan Junior wrote: > diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c > index d5ad6e3..18b817f 100644 > --- a/gdb/linux-tdep.c > +++ b/gdb/linux-tdep.c [...] > @@ -1153,6 +1155,197 @@ linux_corefile_thread_callback (struct thread_info *info, void *data) > return !args->note_data; > } > > +/* Fill the PRPSINFO structure with information about the process being > + debugged. Returns 1 in case of success, 0 for failures. Please note that > + even if the structure cannot be entirely filled (e.g., GDB was unable to > + gather information about the process UID/GID), this function will still > + return 1 since some information was already recorded. It will only return > + 0 iff nothing can be gathered. */ > + > +static int > +linux_fill_prpsinfo (struct elf_internal_prpsinfo *p) > +{ > + /* The filename which we will use to obtain some info about the process. > + We will basically use this to store the `/proc/PID/FILENAME' file. */ > + char filename[100]; > + /* The full name of the program which generated the corefile. */ > + char *fname; > + /* The basename of the executable. */ > + const char *basename; > + /* The arguments of the program. */ > + char *psargs; > + char *infargs; > + /* The contents of `/proc/PID/stat' file. */ > + char *proc_stat, *proc_stat_orig; > + /* The valid states of a process, according to the Linux kernel. */ > + const char valid_states[] = "RSDTZW"; > + /* The program state. */ > + char *prog_state; It could be (also for C++ compliance with its overloaded strchr): const char *prog_state; > + /* The state of the process. */ > + char pr_sname; > + /* The PID of the program which generated the corefile. */ > + pid_t pid; > + /* Process flags. */ > + unsigned int pr_flag; > + /* Process nice value. */ > + long pr_nice; > + /* The stat of the `/proc/PID/stat' file. */ > + struct stat proc_st; > + /* The number of fields read by `sscanf'. */ > + int n_fields = 0; > + /* Cleanups. */ > + struct cleanup *c; > + int i; > + volatile struct gdb_exception ex; > + > + gdb_assert (p != NULL); > + > + /* Obtaining PID and filename. */ > + pid = ptid_get_pid (inferior_ptid); > + xsnprintf (filename, sizeof (filename), "/proc/%u/cmdline", pid); > + fname = target_fileio_read_stralloc (filename); > + > + if (fname == NULL || *fname == '\0') > + { > + /* No program name was read, so we won't be able to retrieve more > + information about the process. */ > + xfree (fname); > + return 0; > + } > + > + c = make_cleanup (xfree, fname); > + memset (p, 0, sizeof (*p)); > + > + /* Obtaining the file stat as well. */ > + errno = 0; > + if (stat (filename, &proc_st) != 0) A nitpick but a failed syscall has to initialize ERRNO so the explicit initialization is not needed. (It is used for example for ptrace(PTRACE_PEEKTEXT) where one cannot find out whether the syscall was successful or not from the ptrace return value.) > + { > + warning (_("Could not stat file `%s': %s"), filename, > + safe_strerror (errno)); > + p->pr_uid = 0; > + p->pr_gid = 0; > + } > + else > + { > + p->pr_uid = proc_st.st_uid; > + p->pr_gid = proc_st.st_gid; > + } > + > + /* Defining the PID. */ > + p->pr_pid = pid; > + > + /* Copying the program name. Only the basename matters. */ > + basename = lbasename (fname); > + strncpy (p->pr_fname, basename, sizeof (p->pr_fname)); > + p->pr_fname[sizeof (p->pr_fname) - 1] = '\0'; > + > + /* Generating and copying the program's arguments. `get_inferior_args' > + may throw, but we want to continue the execution anyway. */ > + TRY_CATCH (ex, RETURN_MASK_ERROR) > + { > + infargs = get_inferior_args (); > + } > + > + if (ex.reason < 0) > + { > + warning (_("Could not obtain inferior's arguments.")); You could print also ex.message here. > + infargs = NULL; > + } > + > + psargs = xstrdup (fname); > + if (infargs != NULL) > + psargs = reconcat (psargs, psargs, " ", infargs, NULL); > + > + make_cleanup (xfree, psargs); > + > + strncpy (p->pr_psargs, psargs, sizeof (p->pr_psargs)); > + p->pr_psargs[sizeof (p->pr_psargs) - 1] = '\0'; > + > + xsnprintf (filename, sizeof (filename), "/proc/%u/stat", pid); Originally: # > + xsnprintf (filename, sizeof (filename), "/proc/%d/stat", pid); # # pid is pid_t, I believe on some systems it may be incompatible with %d. Sorry I did mean to use: xsnprintf (filename, sizeof (filename), "/proc/%d/stat", (int) pid); > + proc_stat = target_fileio_read_stralloc (filename); Moving make_cleanup (xfree, proc_stat); here will not need xfree (proc_stat); below. > + > + if (proc_stat == NULL || *proc_stat == '\0') > + { > + /* Despite being unable to read more information about the process, we > + return 1 here because at least we have its command line, PID and > + arguments. */ > + xfree (proc_stat); > + do_cleanups (c); > + return 1; > + } > + > + proc_stat_orig = proc_stat; > + make_cleanup (xfree, proc_stat_orig); Probably some leftover, it is enough to call: make_cleanup (xfree, proc_stat); PROC_STAT is passed by value, not by reference. > + > + /* Ok, we have the stats. It's time to do a little parsing of the contents > + of the buffer, so that we end up reading what we want. > + > + The following parsing mechanism is strongly based on the information > + generated by the `fs/proc/array.c' file, present in the Linux kernel > + tree. More details about how the information is displayed can be > + obtained by seeing the manpage of proc(5), specifically under the entry > + of `/proc/[pid]/stat'. */ [...] Thanks, Jan