Was doing more code study on the getcwd problem, The coreutils config.hin no more defines any of RAW_DECL_* functions, which where present in 8.15. Can we except more problems because of this?
Thanks & Regards Nagendra.V.S -----Original Message----- From: Joachim Schmitz [mailto:j...@schmitz-digital.de] Sent: Thursday, July 19, 2012 11:24 PM To: 'Paul Eggert' Cc: 10...@debbugs.gnu.org; bug-gnulib@gnu.org; 'Eric Blake'; 'Jim Meyering'; Schmitz, Joachim; V S, Nagendra (Nonstop Filesystems Team) Subject: RE: bug#10305: coreutils-8.14, "rm -r" fails with EBADF > From: Joachim Schmitz [mailto:j...@schmitz-digital.de] > Sent: Wednesday, June 27, 2012 9:25 AM > To: 'Paul Eggert' > Cc: '10...@debbugs.gnu.org'; 'bug-gnulib@gnu.org'; 'Eric Blake'; 'Jim > Meyering' > Subject: RE: bug#10305: coreutils-8.14, "rm -r" fails with EBADF > > > From: Paul Eggert [mailto:egg...@cs.ucla.edu] > > Sent: Wednesday, June 27, 2012 2:49 AM > > To: Joachim Schmitz > > Cc: 10...@debbugs.gnu.org; bug-gnulib@gnu.org; 'Eric Blake'; 'Jim > Meyering' > > Subject: Re: bug#10305: coreutils-8.14, "rm -r" fails with EBADF > > > > On 06/26/2012 09:38 AM, Joachim Schmitz wrote: > > > > > Let me know what you think and where/how you'd do it differently. > > > > The changes mostly look good. The trivial ones we've incorporated > > already. I have some comments on the nontrivial ones (please see below). > > Here are some comments about that patch: > > > --- ./gnu/dirfd.c.orig 2011-03-12 03:14:28.000000000 -0600 > > > +++ ./gnu/dirfd.c 2012-06-25 02:55:09.000000000 -0500 > > > ... > > > +#ifdef __TANDEM > > > +# include <unistd.h> /* for _gl_fnum2dt(), needed in C99 mode */ > > > +#endif > > > > Shouldn't that be "_gl_fnum2fd"? > > Yes, of course, stupid typo. > > > More important, doesn't the declaration of _gl_fnum2fd belong better > > in dirent.h, not unistd.h? Among other things, that would mean the > > above change can be omitted. > > My attempts to integrate this into coreutils-8.17 seem to indicate > that this function and a couple more are used elsewhere too Looks like > there closedir.o needs _gl_ungerister_fnum(), dirfd.c needs > _gl_fnum2ds() and opendir.c needs _gl_register_fnum(). Furthermore that function needs access to fnum2fdmap, which is local to fchdir.c > > > int > > > dirfd (DIR *dir_p) > > > { > > > int fd = DIR_TO_FD (dir_p); > > > if (fd == -1) > > > errno = ENOTSUP; > > > +#ifdef __TANDEM > > > + fd = _gl_fnum2fd(fd); > > > +#endif > > > > This might be cleaner if DIR_TO_FD invoked _gl_fnum2fd directly. > > That way, dirfd.c could be left alone. (Or perhaps not; I don't > > understand the code that well.) > > Won't that make that macro too complicated? I believe it would. Prove me wrong ;-) > > > + char fnum; /* 'y' or 'n', actually a bool */ > > > > Why not use 1 and 0? That's far more typical for boolean values, > > and generates better code. Done, revealed a bad bug in our code, fixed now too. > Maybe we could also make it a short and place fnum right there? That > would change the implementation quite a bit though, but might make it leaner > too. > And the comment > /* FIXME - add a DIR* member to make dirfd possible on mingw? */ > Indicates that there is a change pending for a similar purpose on a different > platform... Ok now... After quite a few iterations and with integrating the things you already fixed for the next releases of gnulib, coreuitls and tar, we finally got it running fine for coreutils-8.15. Doing the equivalent changes (not 100% identical, the coreutils resp. gnulib code had changed and reshuffled quite a bit) lead us into an endless recursion (well as endless as the stack could grow without aborting). After quite some effort in debugging, Nagendra finally found the culprit: in 8.15 configure checks for several functions to be declared without a macro, one of them is getcwd(), and then puts a #define HAVE_RAW_DECL_GETCWD into config.h. Coreutils-8.17 doesn't have that check anymore, hence doesn't set that define and then in getcwd.c enters that endless recursion loop: getcwd(), openat(), rpl_open(), get_name(), getcwd() .... Our (temp.?) fix is to add an "|| __TANDEM" to lib/getcwd.c, line 138. Find the entire patch we use attached. Some more comments on these: We had to hop thru several loops to get getmntent() implemented for NonStop, you may or may not be interested in the details. It is very system specific. Part of the stuff is outside this patch in our own GNUlib-like library (called floss, Freeware Look for Open System Services). Maybe eventually this gets added to our system libs, who knows... The patch to lib/i-ring.h is needed for the fts-functions to recur deeper than 4 levels, at least on NonStop. It is not really fully understood why that is (not by me at least), but that change cured the symptom ;-). Probably doesn't need to be quit as big... The patch to lib/readutmp.h is ugly but needed, at least for now, ignore it, unless you have a better idea. We don't have utmp and friends at all. This is a bit work in progress, probably falls into the same category as getmntent(). The patches reg. NO_UID and NO_GID might be better done someplace else, or not at all, I'm sure whether they are worth the effort. I've disable a bit of apparently dead code in src/remove.c I'm not sure what's wrong with src/stat.c, but our compiler throws up on the original code. Might well be a bug in our compiler? Same for src/system.h, I can't get it thru the compiler without that hack, but haven't been able to find out why. The patch in src/su.c is not really needed, we can't use this su anyway, as our authentication works differently. But the DEFAULT_USER being "root" might be worth getting added to root-uid.h, possibly? In src/uptime.c it gets pretty system specific (but not as deeply as getmntent), don't know whether you're interested in this, but I included it here anyway. I've tried to fix some tests, split/filter and touch/not-owner. Some more tests are failing for no good reason, but a) I haven't yet found why they fail nor how to fix it and b) never check for error conditions you can't handle, so I just don't care about these test too much ;-) As mentioned further above some patches are taken what you already did to coreutils, gnulib and tar. I've removed a few hunks out of the patch, this may result in warnings by patch about an offset, but I guess you wouldn't apply them unaltered anyway. Wherever you see room for improvement or have more questions, we're open, just let Nagendra and me know (and yes, schm...@hp.com and j...@schmitz-digital.de is the same person, me, just in different roles, business and private, but feel free to pick your choice) Bye, Jojo
