Re: module 'fts-lgpl' not complete

2006-02-02 Thread Jim Meyering
Bruno Haible [EMAIL PROTECTED] wrote:
 Regarding the second patch, I see no explanation for why it
 makes such a fundamental change (not appending `.'?).

 Actually the end of that function was a bit incomplete. It should probably
 look like this:
...

Thanks for the suggestion.
I've applied a similar patch for coreutils.
One difference is the use of ENOTDIR rather than EINVAL,
since that what lstat does for names like non-dir/..

I don't particularly like using stat to simulate lstat, but
in this case it seems worthwhile and safe.
I'll propagate this change to gnulib once it's undergone a little testing.

2006-02-02  Jim Meyering  [EMAIL PROTECTED]

Eliminate the unwelcome (albeit unlikely) possibility of xmalloc
failure on deficient systems, and simplify gnulib lgpl dependencies.
* lstat.c (rpl_lstat): Rewrite to use stat() in place of the
xmalloc/lstat combination.  Based on a patch from Bruno Haible.


Index: lib/lstat.c
===
RCS file: /fetish/cu/lib/lstat.c,v
retrieving revision 1.10
retrieving revision 1.11
diff -c -r1.10 -r1.11
*** lib/lstat.c 22 Sep 2005 06:05:39 -  1.10
--- lib/lstat.c 2 Feb 2006 21:25:06 -   1.11
***
*** 1,6 
  /* Work around a bug of lstat on some systems
  
!Copyright (C) 1997, 1998, 1999, 2000, 2001, 2002, 2003, 2004, 2005 Free
 Software Foundation, Inc.
  
 This program is free software; you can redistribute it and/or modify
--- 1,6 
  /* Work around a bug of lstat on some systems
  
!Copyright (C) 1997, 1998, 1999, 2000, 2001, 2002, 2003, 2004, 2005, 2006 
Free
 Software Foundation, Inc.
  
 This program is free software; you can redistribute it and/or modify
***
*** 30,57 
  
  #include sys/types.h
  #include sys/stat.h
- #include stdlib.h
  #include string.h
  
  #include stat-macros.h
- #include xalloc.h
  
  /* lstat works differently on Linux and Solaris systems.  POSIX (see
!`pathname resolution' in the glossary) requires that programs like `ls'
!take into consideration the fact that FILE has a trailing slash when
!FILE is a symbolic link.  On Linux systems, the lstat function already
!has the desired semantics (in treating `lstat(symlink/,sbuf)' just like
!`lstat(symlink/.,sbuf)', but on Solaris it does not.
  
 If FILE has a trailing slash and specifies a symbolic link,
!then append a `.' to FILE and call lstat a second time.  */
  
  int
  rpl_lstat (const char *file, struct stat *sbuf)
  {
size_t len;
-   char *new_file;
- 
int lstat_result = lstat (file, sbuf);
  
if (lstat_result != 0 || !S_ISLNK (sbuf-st_mode))
--- 30,57 
  
  #include sys/types.h
  #include sys/stat.h
  #include string.h
+ #include errno.h
  
  #include stat-macros.h
  
  /* lstat works differently on Linux and Solaris systems.  POSIX (see
!`pathname resolution' in the glossary) requires that programs like
!`ls' take into consideration the fact that FILE has a trailing slash
!when FILE is a symbolic link.  On Linux and Solaris 10 systems, the
!lstat function already has the desired semantics (in treating
!`lstat (symlink/, sbuf)' just like `lstat (symlink/., sbuf)',
!but on Solaris 9 and earlier it does not.
  
 If FILE has a trailing slash and specifies a symbolic link,
!then use stat() to get more info on the referent of FILE.
!If the referent is a non-directory, then set errno to ENOTDIR
!and return -1.  Otherwise, return stat's result.  */
  
  int
  rpl_lstat (const char *file, struct stat *sbuf)
  {
size_t len;
int lstat_result = lstat (file, sbuf);
  
if (lstat_result != 0 || !S_ISLNK (sbuf-st_mode))
***
*** 59,77 
  
len = strlen (file);
if (len == 0 || file[len - 1] != '/')
! return lstat_result;
  
/* FILE refers to a symbolic link and the name ends with a slash.
!  Append a `.' to FILE and repeat the lstat call.  */
! 
!   /* Add one for the `.' we'll append, and one more for the trailing NUL.  */
!   new_file = xmalloc (len + 1 + 1);
!   memcpy (new_file, file, len);
!   new_file[len] = '.';
!   new_file[len + 1] = 0;
! 
!   lstat_result = lstat (new_file, sbuf);
!   free (new_file);
  
!   return lstat_result;
  }
--- 59,80 
  
len = strlen (file);
if (len == 0 || file[len - 1] != '/')
! return 0;
  
/* FILE refers to a symbolic link and the name ends with a slash.
!  Call stat() to get info about the link's referent.  */
  
!   /* If stat fails, then we do the same.  */
!   if (stat (file, sbuf) != 0)
! return -1;
! 
!   /* If FILE references a directory, return 0.  */
!   if (S_ISDIR (sbuf-st_mode))
! return 0;
! 
!   /* Here, we know stat succeeded and FILE references a non-directory.
!  But it was specified via a name including a trailing slash.
!  Fail with errno set to ENOTDIR to indicate the contradiction.  */
!   errno = ENOTDIR;
!   

Re: module 'fts-lgpl' not complete

2006-01-25 Thread Jim Meyering
Bruno Haible [EMAIL PROTECTED] wrote:
 Building the module 'fts-lgpl' on Linux/glibc fails like this:

 gcc -DHAVE_CONFIG_H -I. -I/packages/megatestdir/fts-lgpl/lib -I.. -g -O2 
 -c /packages/megatestdir/fts-lgpl/lib/fts.c
 /packages/megatestdir/fts-lgpl/lib/fts.c:75:20: lstat.h: No such file or 
 directory

 The reason is that the 'lstat' module is GPL.

Hi Bruno,

 Jim, would you agree to put the 'lstat' module under LGPL if we cut its
 dependency from the 'xalloc' module?

Sounds reasonable.

 Actually, using allocsa instead of
 xmalloc will also speed it up. What do you think about these two alternative
 patches?

The first one looks ok, but just using malloc would be simpler.
IMHO, performance shouldn't be a concern here.  This is a replacement
function to work around deficient systems, after all.

Regarding the second patch, I see no explanation for why it
makes such a fundamental change (not appending `.'?).


___
bug-gnulib mailing list
bug-gnulib@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-gnulib


Re: module 'fts-lgpl' not complete

2006-01-25 Thread Bruno Haible
Hi Jim,

 Regarding the second patch, I see no explanation for why it
 makes such a fundamental change (not appending `.'?).

Actually the end of that function was a bit incomplete. It should probably
look like this:

/* FILE refers to a symbolic link and the name ends with a slash.
   Call stat() to get info about the link's destination.  */
  
lstat_result = stat (file, sbuf);
if (lstat_result = 0  !S_ISDIR (sbuf-st_mode))
  {
errno = EINVAL;
return -1;
  }
else
  return lstat_result;

The idea is that if FILE is a symlink and ends in '/', we want to inspect
the inode of the target directory of the link, i.e. follow the symlink
chain. lstat(concat(FILE,.)) does this, but stat(FILE) does this as well,
and needs no intermediate memory allocation. Or am I overlooking something
obvious?

Bruno



___
bug-gnulib mailing list
bug-gnulib@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-gnulib