Discussion:
glob: simplify code
(too old to reply)
Bruno Haible
2017-07-06 21:39:57 UTC
Permalink
Raw Message
Hi Paul,

This proposed patch makes a couple of lines of code easier to read and review,
to me. Do you also find that it is a simplification, worth committing?
(Context diff below.)


2017-07-06 Bruno Haible <***@clisp.org>

glob: Simplify code.
* lib/glob.c (glob): Use a local variable instead of
pglob->gl_pathv[newcount]. Don't write a useless NULL pointer into
memory.

diff --git a/lib/glob.c b/lib/glob.c
*** a/lib/glob.c
--- b/lib/glob.c
***************
*** 1084,1094 ****

if (flags & GLOB_MARK)
{
! char *p;
! pglob->gl_pathv[newcount] = malloc (dirlen + 2);
! if (pglob->gl_pathv[newcount] == NULL)
goto nospace;
! p = mempcpy (pglob->gl_pathv[newcount], dirname, dirlen);
p[0] = '/';
p[1] = '\0';
if (__glibc_unlikely (malloc_dirname))
--- 1084,1094 ----

if (flags & GLOB_MARK)
{
! char *p = malloc (dirlen + 2);
! if (p == NULL)
goto nospace;
! pglob->gl_pathv[newcount] = p;
! p = mempcpy (p, dirname, dirlen);
p[0] = '/';
p[1] = '\0';
if (__glibc_unlikely (malloc_dirname))
***************
*** 1100,1108 ****
pglob->gl_pathv[newcount] = dirname;
else
{
! pglob->gl_pathv[newcount] = strdup (dirname);
! if (pglob->gl_pathv[newcount] == NULL)
goto nospace;
}
}
pglob->gl_pathv[++newcount] = NULL;
--- 1100,1109 ----
pglob->gl_pathv[newcount] = dirname;
else
{
! char *p = strdup (dirname);
! if (p == NULL)
goto nospace;
+ pglob->gl_pathv[newcount] = p;
}
}
pglob->gl_pathv[++newcount] = NULL;
Paul Eggert
2017-07-06 21:54:18 UTC
Permalink
Raw Message
Although I also find the code more readable with your change, I worry that it
will cause gnulib glob.c to diverge from glibc glob.c for stylistic reasons,
which is a minus.
Bruno Haible
2017-07-06 22:18:23 UTC
Permalink
Raw Message
Post by Paul Eggert
Although I also find the code more readable with your change, I worry that it
will cause gnulib glob.c to diverge from glibc glob.c for stylistic reasons,
which is a minus.
OK, I discard the patch.

Bruno

Loading...