From owner-freebsd-audit Sun Aug 12 11:42:27 2001 Delivered-To: freebsd-audit@freebsd.org Received: from mail.musha.org (daemon.musha.org [61.122.44.178]) by hub.freebsd.org (Postfix) with ESMTP id 93E2337B401; Sun, 12 Aug 2001 11:42:21 -0700 (PDT) (envelope-from knu@iDaemons.org) Received: from archon.local.idaemons.org (archon.local.idaemons.org [192.168.1.32]) by mail.musha.org (Postfix) with ESMTP id D94104D833; Mon, 13 Aug 2001 03:42:19 +0900 (JST) Date: Mon, 13 Aug 2001 03:42:19 +0900 Message-ID: <86snex15o4.wl@archon.local.idaemons.org> From: "Akinori MUSHA" To: Mike Barcroft Cc: audit@FreeBSD.org Subject: Re: adding -P option to pkg_delete(1) In-Reply-To: <20010812140750.B29132@coffee.q9media.com> References: <86elqphctp.wl@archon.local.idaemons.org> <86d761hijs.wl@archon.local.idaemons.org> <20010812140750.B29132@coffee.q9media.com> User-Agent: Wanderlust/2.7.1 (Too Funky) SEMI/1.14.3 (Ushinoya) FLIM/1.14.3 (=?ISO-8859-1?Q?Unebigory=F2mae?=) APEL/10.3 MULE XEmacs/21.1 (patch 14) (Cuyahoga Valley) (i386--freebsd) Organization: Associated I. Daemons X-PGP-Public-Key: finger knu@FreeBSD.org X-PGP-Fingerprint: 081D 099C 1705 861D 4B70 B04A 920B EFC7 9FD9 E1EE MIME-Version: 1.0 (generated by SEMI 1.14.3 - "Ushinoya") Content-Type: text/plain; charset=US-ASCII Sender: owner-freebsd-audit@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG Thanks for the review, At Sun, 12 Aug 2001 14:07:50 -0400, Mike Barcroft wrote: > > /* > > + * Check if the given filename looks like a shared library. > > + */ > > +static Boolean > > +is_shlib(char *filename) > > +{ > > + char *p, *q; > > + > > + /* basename */ > > + if (NULL != (p = strrchr(filename, '/'))) > > + p++; > > + else > > + p = filename; > > + > > + /* empty filename or dotfile? */ > > + if (*p == '\0' || *p == '.') > > + return FALSE; > > + > > + /* do "strrstr" for .so */ > > + if (NULL == (q = strstr(p + 1, ".so"))) > > + return FALSE; > > + while (NULL != (p = strstr(q += 3, ".so"))) > > + q = p; > > + > > + /* skip version numbers */ > > + while (*q) { > > + if (*q != '.' || !isdigit(*++q)) > > + return FALSE; > > + while (isdigit(*++q)) > > + ; > > + } > > + > > + return TRUE; > > +} > [...] > > This could probably be written better, so that you don't have to walk > filename so many times. Yes, alternatively you could write it as follows, for example: /* [^\/]\.so(\.\d+)*$ */ static Boolean is_shlib(char *filename) { char *p; p = strrchr(filename, 's'); if (p == NULL || p[1] != 'o' || p - filename < 2 || p[-1] != '.' || p[-2] == '/') return FALSE; p += 2; /* skip version numbers */ while (*p) { if (*p != '.' || !isdigit(*++p)) return FALSE; while (isdigit(*++p)) ; } return TRUE; } (But I don't like this ;) > [...] > > - sprintf(tmp, "%s/%s", Where, p->name); > > + sprintf(tmp, "%s/%s", Where, last_file); > [...] > > I don't see any checks to ensure that this won't overflow tmp, so you > should probably use snprintf(3) instead. Indeed, and that kind of code is everywhere in the pkg_install sources. They'll need overall audit. > The rest of the patch looks reasonable, but I agree with Sheldon that > this shouldn't be merged into -STABLE until after 4.4-RELEASE. I see. -- / /__ __ Akinori.org / MUSHA.org / ) ) ) ) / FreeBSD.org / Ruby-lang.org Akinori MUSHA aka / (_ / ( (__( @ iDaemons.org / and.or.jp "Freeze this moment a little bit longer, make each impression a little bit stronger.. Experience slips away -- Time stand still" To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message