Re: [PATCH] Fix glob() function

Hi Jordi,

the patch looks good, thank you.  Can you please send it as a git patch
created with `git format-patch' and a nice commit message?


Thanks,
Corinna


On Aug 20 08:03, Jordi Sanfeliu wrote:
> Find the patch attached, I hope is OK this time.
> Thanks.
> 
> --
> Jordi Sanfeliu
> FIBRANET Network Services Provider
> https://www.fibranet.cat
> 
> 
> On Mon, 19 Aug 2024, Corinna Vinschen wrote:
> 
> > Hi Jordi,
> > 
> > On Aug 17 17:38, Jordi Sanfeliu wrote:
> > > Hello,
> > > 
> > > While porting 'mandoc' [1] to my hobbyOS FiwixOS, I've discovered that the
> > > glob() function in Newlib always returns zero (success) even when the
> > > pathname was not found.
> > > 
> > > I was comparing the file 'libc/posix/glob.c' with the one from Apple [2] and
> > > then I applied the following patch:
> > > 
> > > diff --git a/newlib/libc/posix/glob.c b/newlib/libc/posix/glob.c
> > > index 5e6c2fcba..0347979de 100644
> > > --- a/newlib/libc/posix/glob.c
> > > +++ b/newlib/libc/posix/glob.c
> > > @@ -502,11 +502,14 @@ glob0(pattern, pglob, limit)
> > >   	 * and the pattern did not contain any magic characters
> > >   	 * GLOB_NOMAGIC is there just for compatibility with csh.
> > >   	 */
> > > -	if (pglob->gl_pathc == oldpathc &&
> > > -	    ((pglob->gl_flags & GLOB_NOCHECK) ||
> > > -	      ((pglob->gl_flags & GLOB_NOMAGIC) &&
> > > -	       !(pglob->gl_flags & GLOB_MAGCHAR))))
> > > -		return(globextend(pattern, pglob, limit));
> > > +	if (pglob->gl_pathc == oldpathc) {
> > > +	    if (((pglob->gl_flags & GLOB_NOCHECK) ||
> > > +	        ((pglob->gl_flags & GLOB_NOMAGIC) &&
> > > +	        !(pglob->gl_flags & GLOB_MAGCHAR))))
> > > +		    return(globextend(pattern, pglob, limit));
> > > +	    else
> > 
> > This is correct and as upstream, but...
> > 
> > > +		    return(3);	/* GLOB_NOMATCH */
> > 
> > ...this isn't.  Check with libc/include/glob.h.  Your patch should
> > add a
> > 
> >  #define GLOB_NOMATCH (-3)
> > 
> > to glob.h instead.
> > 
> > Also, check your whitespaces.  I'm getting quite a few warnings from
> > `git am':
> > 
> >  Applying: Fix glob() function
> >  .git/rebase-apply/patch:6: space before tab in indent.
> > 	   * and the pattern did not contain any magic characters
> >  .git/rebase-apply/patch:7: space before tab in indent.
> > 	   * GLOB_NOMAGIC is there just for compatibility with csh.
> >  .git/rebase-apply/patch:8: space before tab in indent.
> > 	   */
> >  .git/rebase-apply/patch:22: space before tab in indent.
> > 	  else if (!(pglob->gl_flags & GLOB_NOSORT))
> >  .git/rebase-apply/patch:23: space before tab in indent.
> > 		  qsort(pglob->gl_pathv + pglob->gl_offs + oldpathc,
> >  warning: squelched 1 whitespace error
> >  warning: 6 lines add whitespace errors.
> > 
> > 
> > Thanks,
> > Corinna
> > 
> > 

> diff --git a/newlib/libc/include/glob.h b/newlib/libc/include/glob.h
> index 7a300e69d..c14840cf0 100644
> --- a/newlib/libc/include/glob.h
> +++ b/newlib/libc/include/glob.h
> @@ -80,6 +80,7 @@ typedef struct {
>  
>  #define	GLOB_NOSPACE	(-1)	/* Malloc call failed. */
>  #define	GLOB_ABEND	(-2)	/* Unignored error. */
> +#define	GLOB_NOMATCH	(-3)	/* No match and GLOB_NOCHECK not set. */
>  
>  __BEGIN_DECLS
>  int	glob(const char *__restrict, int, int (*)(const char *, int), 
> diff --git a/newlib/libc/posix/glob.c b/newlib/libc/posix/glob.c
> index 5e6c2fcba..20eec0263 100644
> --- a/newlib/libc/posix/glob.c
> +++ b/newlib/libc/posix/glob.c
> @@ -502,11 +502,14 @@ glob0(pattern, pglob, limit)
>  	 * and the pattern did not contain any magic characters
>  	 * GLOB_NOMAGIC is there just for compatibility with csh.
>  	 */
> -	if (pglob->gl_pathc == oldpathc &&
> -	    ((pglob->gl_flags & GLOB_NOCHECK) ||
> -	      ((pglob->gl_flags & GLOB_NOMAGIC) &&
> -	       !(pglob->gl_flags & GLOB_MAGCHAR))))
> -		return(globextend(pattern, pglob, limit));
> +	if (pglob->gl_pathc == oldpathc) {
> +		if ((pglob->gl_flags & GLOB_NOCHECK) ||
> +		    ((pglob->gl_flags & GLOB_NOMAGIC) &&
> +		    !(pglob->gl_flags & GLOB_MAGCHAR)))
> +			return(globextend(pattern, pglob, limit));
> +		else
> +			return(GLOB_NOMATCH);
> +	}
>  	else if (!(pglob->gl_flags & GLOB_NOSORT))
>  		qsort(pglob->gl_pathv + pglob->gl_offs + oldpathc,
>  		    pglob->gl_pathc - oldpathc, sizeof(char *), compare);