Edward O'Callaghan
2009-Nov-25 06:38 UTC
[LLVMdev] [llvm-commits] [llvm] r89765 - in /llvm/trunk: include/llvm/System/Path.h lib/System/Unix/Path.inc lib/System/Win32/Path.inc
G'Day, Following Daniels comments about semantics of the sys::Path API, he has convinced me otherwise as the driver doesn't make or remove directories, so his semantics do indeed make more sense in this context. Fixes applied here; LLVM: http://llvm.org/viewvc/llvm-project?view=rev&revision=89848 Clang: http://llvm.org/viewvc/llvm-project?view=rev&revision=89849 Thanks everyone for the feedback, Edward. 2009/11/24 Edward O'Callaghan <eocallaghan at auroraux.org>:> G'Day Daniel, > >> I would prefer this be Path::isRegularFile, that corresponds to a well >> known Unixism (S_ISREG) and avoids creating new terminology. >> Similarly, I don't think it should do anything else -- checking that >> the path is a directory is something clients can do. > > You know that's about the forth time someone has told me to change the name. :| > Also, isRegularFile would be wrong because it returns false for being > a regular file, that's not we want to be checking for, you got it the > wrong way around. > > I'm not sure what you mean about not checking a directory, if its not > a reg file or a directory then its special. Please see how its being > used on the clang side of this fix. This is a two part fix, one side > on LLVM and the other on Clang. > > Cheers, > Edward. > > 2009/11/24 Daniel Dunbar <daniel at zuster.org>: >> Hi Edward, >> >> >> On Tue, Nov 24, 2009 at 7:19 AM, Edward O'Callaghan >> <eocallaghan at auroraux.org> wrote: >>> Author: evocallaghan >>> Date: Tue Nov 24 09:19:10 2009 >>> New Revision: 89765 >>> >>> URL: http://llvm.org/viewvc/llvm-project?rev=89765&view=rev >>> Log: >>> Provide Path::isSpecialFile interface for PR5568. >>> >>> Modified: >>> llvm/trunk/include/llvm/System/Path.h >>> llvm/trunk/lib/System/Unix/Path.inc >>> llvm/trunk/lib/System/Win32/Path.inc >>> >>> Modified: llvm/trunk/include/llvm/System/Path.h >>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/System/Path.h?rev=89765&r1=89764&r2=89765&view=diff >>> >>> =============================================================================>>> --- llvm/trunk/include/llvm/System/Path.h (original) >>> +++ llvm/trunk/include/llvm/System/Path.h Tue Nov 24 09:19:10 2009 >>> @@ -380,6 +380,11 @@ >>> /// in the file system. >>> bool canWrite() const; >>> >>> + /// This function checks that what we're trying to work only on a regular file or Dir. >>> + /// Check for things like /dev/null, any block special file, >>> + /// or other things that aren't "regular" files. >>> + bool isSpecialFile() const; >>> + >>> /// This function determines if the path name references an executable >>> /// file in the file system. This function checks for the existence and >>> /// executability (by the current program) of the file. >> >> >> I would prefer this be Path::isRegularFile, that corresponds to a well >> known Unixism (S_ISREG) and avoids creating new terminology. >> Similarly, I don't think it should do anything else -- checking that >> the path is a directory is something clients can do. >> >> - Daniel >> >>> Modified: llvm/trunk/lib/System/Unix/Path.inc >>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/System/Unix/Path.inc?rev=89765&r1=89764&r2=89765&view=diff >>> >>> =============================================================================>>> --- llvm/trunk/lib/System/Unix/Path.inc (original) >>> +++ llvm/trunk/lib/System/Unix/Path.inc Tue Nov 24 09:19:10 2009 >>> @@ -335,7 +335,7 @@ >>> free(pv); >>> return (NULL); >>> } >>> -#endif >>> +#endif // __FreeBSD__ >>> >>> /// GetMainExecutable - Return the path to the main executable, given the >>> /// value of argv[0] from program startup. >>> @@ -454,6 +454,24 @@ >>> } >>> >>> bool >>> +Path::isSpecialFile() const { >>> + // Get the status so we can determine if its a file or directory >>> + struct stat buf; >>> + std::string *ErrStr; >>> + >>> + if (0 != stat(path.c_str(), &buf)) { >>> + MakeErrMsg(ErrStr, path + ": can't get status of file"); >>> + return true; >>> + } >>> + >>> + if (S_ISDIR(buf.st_mode) || S_ISREG(buf.st_mode)) { >>> + return false; >>> + } >>> + >>> + return true; >>> +} >>> + >>> +bool >>> Path::canExecute() const { >>> if (0 != access(path.c_str(), R_OK | X_OK )) >>> return false; >>> @@ -723,7 +741,7 @@ >>> >>> bool >>> Path::eraseFromDisk(bool remove_contents, std::string *ErrStr) const { >>> - // Get the status so we can determin if its a file or directory >>> + // Get the status so we can determine if its a file or directory >>> struct stat buf; >>> if (0 != stat(path.c_str(), &buf)) { >>> MakeErrMsg(ErrStr, path + ": can't get status of file"); >>> >>> Modified: llvm/trunk/lib/System/Win32/Path.inc >>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/System/Win32/Path.inc?rev=89765&r1=89764&r2=89765&view=diff >>> >>> =============================================================================>>> --- llvm/trunk/lib/System/Win32/Path.inc (original) >>> +++ llvm/trunk/lib/System/Win32/Path.inc Tue Nov 24 09:19:10 2009 >>> @@ -357,6 +357,11 @@ >>> return attr != INVALID_FILE_ATTRIBUTES; >>> } >>> >>> +bool >>> +Path::isSpecialFile() const { >>> + return false; >>> +} >>> >>> std::string >>> Path::getLast() const { >>> // Find the last slash >>> >>> >>> _______________________________________________ >>> llvm-commits mailing list >>> llvm-commits at cs.uiuc.edu >>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits >>> >> > > > > -- > -- > Edward O'Callaghan > http://www.auroraux.org/ > eocallaghan at auroraux dot org > --- > () ascii ribbon campaign - against html e-mail > /\ - against microsoft attachments >-- -- Edward O'Callaghan http://www.auroraux.org/ eocallaghan at auroraux dot org --- () ascii ribbon campaign - against html e-mail /\ - against microsoft attachments