[llvm-commits] [llvm] r89765 - in /llvm/trunk: include/llvm/System/Path.h lib/System/Unix/Path.inc lib/System/Win32/Path.inc

[Edward O'Callaghan]

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