Paul Eggert <[EMAIL PROTECTED]> wrote: > I wanted to use path_concat in Bison to fix a porting problem to DOS, > and noticed that path_concat had the same problem: it mishandled the > case where BASE starts with "c:". While fixing this, I noticed a few > other problems: > > * cp, install, and ls all assume that path_concat never returns NULL, > and they can dump core if path_concat fails due to memory exhaustion. > > * There is a function xpath_concat defined in path-concat.c but it's not > declared and nobody uses it. I think it's simpler to have path_concat > always return a nonnull pointer.
I'd prefer to continue the x-prefixing-functions-that-may-exit tradition, but since no application requires the non-exiting version of path_concat, I suppose it can wait and be renamed later if/when the need arises. > * cp.c has a potential integer overflow if the dir name has more than 2 GiB, > and will trash the resulting file name. FYI, there are other problems with copy when file names become very long. The most serious are that it's limited by PATH_MAX and that it uses alloca for things that might get bigger than 4k (long file names). There's already a note (I've just made it a FIXME) to convert copy.c to fix both problems by switching to use fts. > * Only nohup.c passes a NULL pointer as the first argument to > path_concat, but it is a glitch, and it causes nohup to try to open > the same file twice. I rewrote nohup to avoid the glitch, so that > path_concat can now assume that its first argument is non NULL. > > * There is a (perhaps theoretical?) problem if some standard header > defines mempcpy as a macro, but there is no mempcpy function. > > * Sometimes more than one slash is inserted between the dir and the base, > if they started with multiple slashes there. This is just a minor annoyance > but while we're in the neighborhood.... > > Here is a patch. > > Index: ChangeLog > =================================================================== > RCS file: /home/meyering/coreutils/cu/ChangeLog,v > retrieving revision 1.976 > diff -p -u -r1.976 ChangeLog > --- ChangeLog 2 Jul 2004 17:01:30 -0000 1.976 > +++ ChangeLog 2 Jul 2004 21:02:30 -0000 > @@ -1,3 +1,16 @@ > +2004-07-02 Paul Eggert <[EMAIL PROTECTED]> > + > + * src/copy.c (copy_dir): Assume path_concat returns non-NULL. > + * src/cp.c (do_copy): Likewise. > + * src/mv.c (movefile): Likewise. > + > + * src/cp.c (make_path_private): 2nd arg is now size_t, not int, > + to avoid problem when path_concat dir name is longer than 2 GiB (!). > + > + * src/nohup.c (main): Don't pass NULL first argument to path_concat. > + This cleans up the semantics a bit, as we no longer try to open the > + same file twice. Applied. Thanks! Only one nit. I'm not sure what you intended by the `cd DIR; cat BASE' part of the comment for the new path_concat. The resulting file name is equivalent to what you would get by running (cd DIR; cat BASE), where BASE is ABASE with any file system prefixes and leading separators removed. _______________________________________________ Bug-coreutils mailing list [EMAIL PROTECTED] http://lists.gnu.org/mailman/listinfo/bug-coreutils
