From owner-svn-src-head@FreeBSD.ORG Mon May 6 10:18:07 2013 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by hub.freebsd.org (Postfix) with ESMTP id 4B8B249D; Mon, 6 May 2013 10:18:07 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from bigwig.baldwin.cx (bigwig.baldwin.cx [IPv6:2001:470:1f11:75::1]) by mx1.freebsd.org (Postfix) with ESMTP id 2888E64C; Mon, 6 May 2013 10:18:07 +0000 (UTC) Received: from ralph.baldwin.cx (pool-173-70-85-244.nwrknj.fios.verizon.net [173.70.85.244]) by bigwig.baldwin.cx (Postfix) with ESMTPSA id 763B5B983; Mon, 6 May 2013 06:18:06 -0400 (EDT) From: John Baldwin To: Mikolaj Golub Subject: Re: svn commit: r250223 - in head: lib/libprocstat sys/kern sys/sys usr.bin/fstat Date: Mon, 6 May 2013 06:17:58 -0400 User-Agent: KMail/1.13.7 (FreeBSD/9.1-STABLE; KDE/4.8.4; amd64; ; ) References: <201305032111.r43LBvZ3040508@svn.freebsd.org> <20130504185419.GA32075@gmail.com> In-Reply-To: <20130504185419.GA32075@gmail.com> MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201305060617.59143.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.2.7 (bigwig.baldwin.cx); Mon, 06 May 2013 06:18:06 -0400 (EDT) Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 06 May 2013 10:18:07 -0000 On Saturday, May 04, 2013 02:54:20 PM Mikolaj Golub wrote: > On Fri, May 03, 2013 at 09:11:57PM +0000, John Baldwin wrote: > > +static int > > +procstat_get_sem_info_kvm(kvm_t *kd, struct filestat *fst, > > + struct semstat *sem, char *errbuf) > > +{ > > + struct ksem ksem; > > + void *ksemp; > > + char *path; > > + int i; > > + > > + assert(kd); > > + assert(sem); > > + assert(fst); > > + bzero(sem, sizeof(*sem)); > > + ksemp = fst->fs_typedep; > > + if (ksemp == NULL) > > + goto fail; > > + if (!kvm_read_all(kd, (unsigned long)ksemp, &ksem, > > + sizeof(struct ksem))) { > > + warnx("can't read ksem at %p", (void *)ksemp); > > + goto fail; > > + } > > + sem->mode = S_IFREG | ksem.ks_mode; > > + sem->value = ksem.ks_value; > > + if (fst->fs_path == NULL && ksem.ks_path != NULL) { > > + path = malloc(MAXPATHLEN); > > + for (i = 0; i < MAXPATHLEN - 1; i++) { > > + if (!kvm_read_all(kd, (unsigned long)ksem.ks_path + i, > > + path + i, 1)) > > + break; > > + if (path[i] == '\0') > > + break; > > + } > > + path[i] = '\0'; > > + if (i == 0) > > + free(path); > > + else > > + fst->fs_path = path; > > + } > > + return (0); > > + > > +fail: > > + snprintf(errbuf, _POSIX2_LINE_MAX, "error"); > > + return (1); > > +} > > I would like to make errbuf optional (for all libprocstat functions > that provide it) adding a check before printing to errbuf: > > if (errbuf != NULL) > snprintf(errbuf, ... > > It looks like there are callers who are not interested in errbuf > content. E.g. currently procstat(1) passes NULL when calling functions > that require errbuf, namely procstat_get_socket_info and > procstat_get_vnode_info, potentially crashing here if an error occurs. > > Do you (anyone) have objections? Oh, no not at all. I just used the shm_info version as my template, so it probably has the same issue as well. -- John Baldwin