[LLVMdev] [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,

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