Sender: rich AT phekda DOT freeserve DOT co DOT uk Message-ID: <3EA56BC4.520C8952@phekda.freeserve.co.uk> Date: Tue, 22 Apr 2003 17:20:20 +0100 From: Richard Dawe X-Mailer: Mozilla 4.77 [en] (X11; U; Linux 2.2.23 i586) X-Accept-Language: de,fr MIME-Version: 1.0 To: djgpp-workers AT delorie DOT com Subject: Re: fstat, fd_props and inventing inodes, revision 3 [PATCH] References: <3791-Sun20Apr2003132207+0300-eliz AT elta DOT co DOT il> Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Reply-To: djgpp-workers AT delorie DOT com Hello. Eli Zaretskii wrote: > > > Date: Sat, 19 Apr 2003 13:14:46 +0100 > > From: "Richard Dawe" > > > > Here's revision 3 of the patch to fstat, to make it use the filename > > from fd_props to generate an inode number, when it can't obtain the inode > > from the SFT. > > Thanks. I have a few minor comments. OK. Thanks for reviewing the patch! > > +/* Find a mapping for the UNC path (\\machine\share) in `src', if one > > + * exists. If one exists, copy the path into `dest' and convert the UNC > > + * to the mapped drive, then return `dest'. Otherwise, just return `src'. > > + */ > > + > > +static const char * > > +find_mapping_for_unc (const char *src, char *dest) > > This sounds like a good function to have in the library, so I'd > suggest to make it a separate module and make the function be extern > (and then its name should begin with 2 underscores). OK, fixed. > > + /* Update the drive mappings, if: we don't have them; a second has > > + * elapsed; the clock has wrapped. We update every second as a basic > > + * caching mechanism, but also to keep up with changes in the mappings. > > This caching, like all caches we have in the library, should have the > standard unexec protection using __bss_count. That is, if the program > is dumped and restarted, the cache should be invalidated. I thought I had already protected it. Its initialisation was carried out by the code called by _get_sft_entry. But I had not realised that _get_sft_entry is not always called for an fstat call. Anyway, making find_mapping_for_unc a library function and moving it into a separate module requires that it has this protection. Fixed. > > + /* Get the file name from the file descriptor properties (fd_props), > > + * if possible, and fix it up. */ > > + if (__get_fd_name(fhandle)) > > + filename = __get_fd_name(fhandle); > > Is it really necessary to call __get_fd_name twice? Sounds like a > waste of cycles, even if it is inlined. OK, fixed. > > +invented using the file name. @code{fstat} can probably use the file name > > +that was used to open the file, when generating the inode. This is done > > +such that the same inode will be generated irrespective of the actual path > > +used to open the file (e.g.: @samp{foo.txt}, @samp{./foo.txt}, > > +@samp{../somedir/foo.txt}). If file names cannot be used, @code{fstat} > > File names should be in @file, not @samp. OK, fixed. > > +@findex fstat AT r{, and inodes} > > +@code{fstat} will now use the file name used to open the file, > > +when inventing inodes. This is done so that the same inode is generated > > +irrespective of the actual file path used to open the file. This also > > +fixes the problem where multiple calls to fstat > > +on the same file descriptor would give different inodes. > > I think it's important to mention that this change makes sure `stat' > and `fstat' report the same inode in most cases. OK. Note that they wouldn't in the presence of UNCs, because stat did not map the UNC to a mapped drive letter. The patched fstat code does that. I've fixed stat to map UNCs to drive letter too. > > + if (drive_mappings[pos] == NULL) > > + drive_mappings[pos] = malloc(sizeof(drive_mapping)); > > As you probably already know, I don't like library functions to call > malloc. Can't we have a static 32-element array for this? Or would > the required storage be unresonably large? > > > +path of the form @file{\\machine\share}. > > IIRC, backslashes have problems when you TeX the manual. I think the > FAQ sources have the solution (search for "\\") which could help you > out. I couldn't find any occurrences of "\\" in the FAQ (2.30). However, the texinfo manual says this about "\\" in @math: " Since `\' is an escape character inside `@math', you can use `@\' to get a literal backslash (`\\' will work in TeX, but you'll get the literal `\\' in Info). `@\' is not defined outside of `@math', since a `\' ordinarily produces a literal `\'." Maybe this is what you are thinking of? A test with texi2dvi (which runs it through TeX AFAICS) & dvips seems to produce a PostScript file with "\\" OK. Thanks! Bye, Rich =] -- Richard Dawe [ http://www.phekda.freeserve.co.uk/richdawe/ ]