[pacman-dev] [PATCH 2/4] Another unneeded NULL check removed

[Dan McGee]

On Tue, Jan 28, 2014 at 11:33 AM, Silvan Jegen <s.jegen at gmail.com> wrote:

> On Tue, Jan 28, 2014 at 10:58:30AM -0600, Dan McGee wrote:
> > On Tue, Jan 28, 2014 at 10:50 AM, Silvan Jegen <s.jegen at gmail.com>
> wrote:
> >
> > > Signed-off-by: Silvan Jegen <s.jegen at gmail.com>
> > > ---
> > >  src/pacman/util.c | 10 ++++------
> > >  1 file changed, 4 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/src/pacman/util.c b/src/pacman/util.c
> > > index 58b0cec..4aaa98f 100644
> > > --- a/src/pacman/util.c
> > > +++ b/src/pacman/util.c
> > > @@ -206,12 +206,10 @@ int rmrf(const char *path)
> > >                         return 1;
> > >                 }
> > >                 for(dp = readdir(dirp); dp != NULL; dp =
> readdir(dirp)) {
> > > -                       if(dp->d_name) {
> > > -                               if(strcmp(dp->d_name, "..") != 0 &&
> > > strcmp(dp->d_name, ".") != 0) {
> > > -                                       char name[PATH_MAX];
> > > -                                       snprintf(name, PATH_MAX,
> "%s/%s",
> > > path, dp->d_name);
> > > -                                       errflag += rmrf(name);
> > > -                               }
> > > +                       if(strcmp(dp->d_name, "..") != 0 &&
> > > strcmp(dp->d_name, ".") != 0) {
> > > +                               char name[PATH_MAX];
> > > +                               snprintf(name, PATH_MAX, "%s/%s", path,
> > > dp->d_name);
> > > +                               errflag += rmrf(name);
> > >
> >
> > I'm failing to see how this one is valid. Please explain? I'm not sure
> the
> > original code is right (we should likely be checking that d_name[0] is !=
> > '\0;), but removing the check completely seems heavy-handed.
>
> My reasoning was very simple. I assume that each directory entry MUST
> have a filename (that only contains a terminating NULL character) so
> checking whether the d_name field of the dirent struct is NULL should
> not be necessary.
>
> If I missed a case where the filename can actually be NULL then just
> ignore this one.
>
> Looking at it more, I think you're right here, especially after I trace
the history of this code. I'm OK with this change. We no longer do it in
the backend either (see commit 1b50223f for some history).

-Dan