zturner added inline comments.

================
Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp:813
+  auto perms =
+      static_cast<llvm::sys::fs::perms>(packet.GetHexMaxU32(false, 
UINT32_MAX));
   if (packet.GetChar() == ',') {
----------------
zturner wrote:
> labath wrote:
> > This doesn't seem right. At the very least I would expect to see a matching 
> > change in the client code (I assume llvm constants don't match whatever we 
> > have used here (?)).
> > 
> > We don't care much about protocol compatibility, but the apple guys might. 
> > If that is the case then we will need some encode/decode functions here.
> AFAICT both LLVM's enumeration and LLDB's enumeration map directly to the 
> unix file permissions bits.
Here is the implementation of `setPermissions` in LLVM:

```

std::error_code setPermissions(const Twine &Path, perms Permissions) {
  SmallString<128> PathStorage;
  StringRef P = Path.toNullTerminatedStringRef(PathStorage);

  if (::chmod(P.begin(), Permissions))
    return std::error_code(errno, std::generic_category());
  return std::error_code();
}
```

So it's literally the same as LLDB's implementation which I've deleted, which 
was:

```
Error FileSystem::SetFilePermissions(const FileSpec &file_spec,
                                     uint32_t file_permissions) {
  Error error;
  if (::chmod(file_spec.GetCString(), file_permissions) != 0)
    error.SetErrorToErrno();
  return error;
}
```


https://reviews.llvm.org/D31089



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to