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?
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);
