On Sun, Dec 29, 2013 at 09:51:28AM -0800, patrick keshishian wrote:
> Hi,
>
> Accidentally deleted this message from my inbox. This is
> a "reconstruction" from mailing list archive.
>
> Suggestion/comment below.
>
> Earlier today:
> > Hi All,
> >
> > From NetBSD:
> > Fix file descriptor leak. Found by cppcheck.
> >
> > Index: src/libexec/ld.so/ldconfig/ldconfig.c
> > ===================================================================
> > RCS file: /cvs/src/libexec/ld.so/ldconfig/ldconfig.c,v
> > retrieving revision 1.31
> > diff -u -p -r1.31 ldconfig.c
> > --- src/libexec/ld.so/ldconfig/ldconfig.c 13 Nov 2013 05:41:42 -0000
> > 1.31
> > +++ src/libexec/ld.so/ldconfig/ldconfig.c 29 Dec 2013 11:53:38 -0000
> > @@ -416,16 +416,16 @@ buildhints(void)
> > if (write(fd, &hdr, sizeof(struct hints_header)) !=
> > sizeof(struct hints_header)) {
> > warn("%s", _PATH_LD_HINTS);
> > - goto out;
> > + goto fdout;
> > }
> > if (write(fd, blist, hdr.hh_nbucket * sizeof(struct hints_bucket)) !=
> > hdr.hh_nbucket * sizeof(struct hints_bucket)) {
> > warn("%s", _PATH_LD_HINTS);
> > - goto out;
> > + goto fdout;
> > }
> > if (write(fd, strtab, strtab_sz) != strtab_sz) {
> > warn("%s", _PATH_LD_HINTS);
> > - goto out;
> > + goto fdout;
> > }
> > if (close(fd) != 0) {
> > warn("%s", _PATH_LD_HINTS);
> > @@ -438,6 +438,8 @@ buildhints(void)
> > }
> >
> > ret = 0;
> > +fdout:
> > + (void)close(fd);
> > out:
> > free(blist);
> > free(strtab);
> >
>
> Why not a simpler diff where the the close() call is moved
> after 'out:' label instead of introducing a new label and
> an additional close() call?
That's equally possible.
I adapted the diff from the NetBSD fix, which uses the additional
label.
I guess that it will boil down to the ldconfig maintainer to decide
which way is better :-)
>
> Like such:
> (disclaimer: not tested, not even compiled.)
>
> Index: ldconfig.c
> ===================================================================
> RCS file: /cvs/obsd/src/libexec/ld.so/ldconfig/ldconfig.c,v
> retrieving revision 1.31
> diff -u -p -u -p -r1.31 ldconfig.c
> --- ldconfig.c 13 Nov 2013 05:41:42 -0000 1.31
> +++ ldconfig.c 29 Dec 2013 15:31:58 -0000
> @@ -322,7 +322,7 @@ hinthash(char *cp, int vmajor, int vmino
> int
> buildhints(void)
> {
> - int strtab_sz = 0, nhints = 0, fd, i, ret = -1, str_index = 0;
> + int strtab_sz = 0, nhints = 0, fd = -1, i, ret = -1, str_index = 0;
> struct hints_bucket *blist;
> struct hints_header hdr;
> struct shlib_list *shp;
> @@ -427,10 +427,6 @@ buildhints(void)
> warn("%s", _PATH_LD_HINTS);
> goto out;
> }
> - if (close(fd) != 0) {
> - warn("%s", _PATH_LD_HINTS);
> - goto out;
> - }
>
> if (rename(tmpfilenam, _PATH_LD_HINTS) != 0) {
> warn("%s", _PATH_LD_HINTS);
> @@ -439,6 +435,8 @@ buildhints(void)
>
> ret = 0;
> out:
> + if (-1 != fd && close(fd) != 0)
> + warn("%s", _PATH_LD_HINTS);
> free(blist);
> free(strtab);
> return (ret);
