[PATCH] D154329: [lldb] Replace llvm::writeFileAtomically with llvm::writeToOutput API.

[Haojian Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG2a579db32a7a: [lldb] Replace llvm::writeFileAtomically with 
llvm::writeToOutput API. (authored by hokein).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154329/new/

https://reviews.llvm.org/D154329

Files:
  lldb/tools/lldb-server/lldb-platform.cpp


Index: lldb/tools/lldb-server/lldb-platform.cpp
===
--- lldb/tools/lldb-server/lldb-platform.cpp
+++ lldb/tools/lldb-server/lldb-platform.cpp
@@ -22,7 +22,6 @@
 #include 
 
 #include "llvm/Support/FileSystem.h"
-#include "llvm/Support/FileUtilities.h"
 #include "llvm/Support/WithColor.h"
 #include "llvm/Support/raw_ostream.h"
 
@@ -103,38 +102,15 @@
 return Status("Failed to create directory %s: %s",
   temp_file_spec.GetPath().c_str(), error.AsCString());
 
-  llvm::SmallString<64> temp_file_path;
-  temp_file_spec.AppendPathComponent("port-file.%%");
-  temp_file_path = temp_file_spec.GetPath();
-
   Status status;
-  if (auto Err =
-  handleErrors(llvm::writeFileAtomically(
-   temp_file_path, file_spec.GetPath(), socket_id),
-   [, _spec](const AtomicFileWriteError ) {
- std::string ErrorMsgBuffer;
- llvm::raw_string_ostream S(ErrorMsgBuffer);
- E.log(S);
-
- switch (E.Error) {
- case atomic_write_error::failed_to_create_uniq_file:
-   status = Status("Failed to create temp file: %s",
-   ErrorMsgBuffer.c_str());
-   break;
- case atomic_write_error::output_stream_error:
-   status = Status("Failed to write to port file.");
-   break;
- case atomic_write_error::failed_to_rename_temp_file:
-   status = Status("Failed to rename file %s to %s: 
%s",
-   ErrorMsgBuffer.c_str(),
-   file_spec.GetPath().c_str(),
-   ErrorMsgBuffer.c_str());
-   break;
- }
-   })) {
-return Status("Failed to atomically write file %s",
-  file_spec.GetPath().c_str());
-  }
+  if (auto Err = llvm::writeToOutput(file_spec.GetPath(),
+ [_id](llvm::raw_ostream ) {
+   OS << socket_id;
+   return llvm::Error::success();
+ }))
+return Status("Failed to atomically write file %s: %s",
+  file_spec.GetPath().c_str(),
+  llvm::toString(std::move(Err)).c_str());
   return status;
 }
 


Index: lldb/tools/lldb-server/lldb-platform.cpp
===
--- lldb/tools/lldb-server/lldb-platform.cpp
+++ lldb/tools/lldb-server/lldb-platform.cpp
@@ -22,7 +22,6 @@
 #include 
 
 #include "llvm/Support/FileSystem.h"
-#include "llvm/Support/FileUtilities.h"
 #include "llvm/Support/WithColor.h"
 #include "llvm/Support/raw_ostream.h"
 
@@ -103,38 +102,15 @@
 return Status("Failed to create directory %s: %s",
   temp_file_spec.GetPath().c_str(), error.AsCString());
 
-  llvm::SmallString<64> temp_file_path;
-  temp_file_spec.AppendPathComponent("port-file.%%");
-  temp_file_path = temp_file_spec.GetPath();
-
   Status status;
-  if (auto Err =
-  handleErrors(llvm::writeFileAtomically(
-   temp_file_path, file_spec.GetPath(), socket_id),
-   [, _spec](const AtomicFileWriteError ) {
- std::string ErrorMsgBuffer;
- llvm::raw_string_ostream S(ErrorMsgBuffer);
- E.log(S);
-
- switch (E.Error) {
- case atomic_write_error::failed_to_create_uniq_file:
-   status = Status("Failed to create temp file: %s",
-   ErrorMsgBuffer.c_str());
-   break;
- case atomic_write_error::output_stream_error:
-   status = Status("Failed to write to port file.");
-   break;
- case atomic_write_error::failed_to_rename_temp_file:
-   status = Status("Failed to rename file %s to %s: %s",
-   ErrorMsgBuffer.c_str(),
-   file_spec.GetPath().c_str(),
-   ErrorMsgBuffer.c_str());
-  

[PATCH] D154329: [lldb] Replace llvm::writeFileAtomically with llvm::writeToOutput API.

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

thanks for the review!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154329/new/

https://reviews.llvm.org/D154329

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154329: [lldb] Replace llvm::writeFileAtomically with llvm::writeToOutput API.

[Jonas Devlieghere via Phabricator via cfe-commits]

JDevlieghere accepted this revision.
JDevlieghere added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154329/new/

https://reviews.llvm.org/D154329

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154329: [lldb] Replace llvm::writeFileAtomically with llvm::writeToOutput API.

[Alexey Lapshin via Phabricator via cfe-commits]

avl added a comment.

this LGTM. thanks! Please, wait for approve from Jonas. I think someone from 
lldb needs to check whether new error reporting is OK.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154329/new/

https://reviews.llvm.org/D154329

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154329: [lldb] Replace llvm::writeFileAtomically with llvm::writeToOutput API.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: lldb/tools/lldb-server/lldb-platform.cpp:112
 return Status("Failed to atomically write file %s",
   file_spec.GetPath().c_str());
   return status;

avl wrote:
> probably, it would be better to add error text here?
> 
> ```
> return Status("Failed to atomically write file %s: %s",
>   file_spec.GetPath().c_str(), 
> toString(std::move(Err)).c_str());
> ```
good idea.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154329/new/

https://reviews.llvm.org/D154329

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154329: [lldb] Replace llvm::writeFileAtomically with llvm::writeToOutput API.

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 536735.
hokein marked an inline comment as done.
hokein added a comment.

address a comment.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154329/new/

https://reviews.llvm.org/D154329

Files:
  lldb/tools/lldb-server/lldb-platform.cpp


Index: lldb/tools/lldb-server/lldb-platform.cpp
===
--- lldb/tools/lldb-server/lldb-platform.cpp
+++ lldb/tools/lldb-server/lldb-platform.cpp
@@ -22,7 +22,6 @@
 #include 
 
 #include "llvm/Support/FileSystem.h"
-#include "llvm/Support/FileUtilities.h"
 #include "llvm/Support/WithColor.h"
 #include "llvm/Support/raw_ostream.h"
 
@@ -103,38 +102,15 @@
 return Status("Failed to create directory %s: %s",
   temp_file_spec.GetPath().c_str(), error.AsCString());
 
-  llvm::SmallString<64> temp_file_path;
-  temp_file_spec.AppendPathComponent("port-file.%%");
-  temp_file_path = temp_file_spec.GetPath();
-
   Status status;
-  if (auto Err =
-  handleErrors(llvm::writeFileAtomically(
-   temp_file_path, file_spec.GetPath(), socket_id),
-   [, _spec](const AtomicFileWriteError ) {
- std::string ErrorMsgBuffer;
- llvm::raw_string_ostream S(ErrorMsgBuffer);
- E.log(S);
-
- switch (E.Error) {
- case atomic_write_error::failed_to_create_uniq_file:
-   status = Status("Failed to create temp file: %s",
-   ErrorMsgBuffer.c_str());
-   break;
- case atomic_write_error::output_stream_error:
-   status = Status("Failed to write to port file.");
-   break;
- case atomic_write_error::failed_to_rename_temp_file:
-   status = Status("Failed to rename file %s to %s: 
%s",
-   ErrorMsgBuffer.c_str(),
-   file_spec.GetPath().c_str(),
-   ErrorMsgBuffer.c_str());
-   break;
- }
-   })) {
-return Status("Failed to atomically write file %s",
-  file_spec.GetPath().c_str());
-  }
+  if (auto Err = llvm::writeToOutput(file_spec.GetPath(),
+ [_id](llvm::raw_ostream ) {
+   OS << socket_id;
+   return llvm::Error::success();
+ }))
+return Status("Failed to atomically write file %s: %s",
+  file_spec.GetPath().c_str(),
+  llvm::toString(std::move(Err)).c_str());
   return status;
 }
 


Index: lldb/tools/lldb-server/lldb-platform.cpp
===
--- lldb/tools/lldb-server/lldb-platform.cpp
+++ lldb/tools/lldb-server/lldb-platform.cpp
@@ -22,7 +22,6 @@
 #include 
 
 #include "llvm/Support/FileSystem.h"
-#include "llvm/Support/FileUtilities.h"
 #include "llvm/Support/WithColor.h"
 #include "llvm/Support/raw_ostream.h"
 
@@ -103,38 +102,15 @@
 return Status("Failed to create directory %s: %s",
   temp_file_spec.GetPath().c_str(), error.AsCString());
 
-  llvm::SmallString<64> temp_file_path;
-  temp_file_spec.AppendPathComponent("port-file.%%");
-  temp_file_path = temp_file_spec.GetPath();
-
   Status status;
-  if (auto Err =
-  handleErrors(llvm::writeFileAtomically(
-   temp_file_path, file_spec.GetPath(), socket_id),
-   [, _spec](const AtomicFileWriteError ) {
- std::string ErrorMsgBuffer;
- llvm::raw_string_ostream S(ErrorMsgBuffer);
- E.log(S);
-
- switch (E.Error) {
- case atomic_write_error::failed_to_create_uniq_file:
-   status = Status("Failed to create temp file: %s",
-   ErrorMsgBuffer.c_str());
-   break;
- case atomic_write_error::output_stream_error:
-   status = Status("Failed to write to port file.");
-   break;
- case atomic_write_error::failed_to_rename_temp_file:
-   status = Status("Failed to rename file %s to %s: %s",
-   ErrorMsgBuffer.c_str(),
-   file_spec.GetPath().c_str(),
-   ErrorMsgBuffer.c_str());
-   break;
- }
- 

[PATCH] D154329: [lldb] Replace llvm::writeFileAtomically with llvm::writeToOutput API.

[Alexey Lapshin via Phabricator via cfe-commits]

avl added inline comments.



Comment at: lldb/tools/lldb-server/lldb-platform.cpp:112
 return Status("Failed to atomically write file %s",
   file_spec.GetPath().c_str());
   return status;

probably, it would be better to add error text here?

```
return Status("Failed to atomically write file %s: %s",
  file_spec.GetPath().c_str(), 
toString(std::move(Err)).c_str());
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154329/new/

https://reviews.llvm.org/D154329

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154329: [lldb] Replace llvm::writeFileAtomically with llvm::writeToOutput API.

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added reviewers: avl, JDevlieghere.
Herald added a project: All.
hokein requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D154329

Files:
  lldb/tools/lldb-server/lldb-platform.cpp


Index: lldb/tools/lldb-server/lldb-platform.cpp
===
--- lldb/tools/lldb-server/lldb-platform.cpp
+++ lldb/tools/lldb-server/lldb-platform.cpp
@@ -22,7 +22,6 @@
 #include 
 
 #include "llvm/Support/FileSystem.h"
-#include "llvm/Support/FileUtilities.h"
 #include "llvm/Support/WithColor.h"
 #include "llvm/Support/raw_ostream.h"
 
@@ -103,38 +102,14 @@
 return Status("Failed to create directory %s: %s",
   temp_file_spec.GetPath().c_str(), error.AsCString());
 
-  llvm::SmallString<64> temp_file_path;
-  temp_file_spec.AppendPathComponent("port-file.%%");
-  temp_file_path = temp_file_spec.GetPath();
-
   Status status;
-  if (auto Err =
-  handleErrors(llvm::writeFileAtomically(
-   temp_file_path, file_spec.GetPath(), socket_id),
-   [, _spec](const AtomicFileWriteError ) {
- std::string ErrorMsgBuffer;
- llvm::raw_string_ostream S(ErrorMsgBuffer);
- E.log(S);
-
- switch (E.Error) {
- case atomic_write_error::failed_to_create_uniq_file:
-   status = Status("Failed to create temp file: %s",
-   ErrorMsgBuffer.c_str());
-   break;
- case atomic_write_error::output_stream_error:
-   status = Status("Failed to write to port file.");
-   break;
- case atomic_write_error::failed_to_rename_temp_file:
-   status = Status("Failed to rename file %s to %s: 
%s",
-   ErrorMsgBuffer.c_str(),
-   file_spec.GetPath().c_str(),
-   ErrorMsgBuffer.c_str());
-   break;
- }
-   })) {
+  if (auto Err = llvm::writeToOutput(file_spec.GetPath(),
+ [_id](llvm::raw_ostream ) {
+   OS << socket_id;
+   return llvm::Error::success();
+ }))
 return Status("Failed to atomically write file %s",
   file_spec.GetPath().c_str());
-  }
   return status;
 }
 


Index: lldb/tools/lldb-server/lldb-platform.cpp
===
--- lldb/tools/lldb-server/lldb-platform.cpp
+++ lldb/tools/lldb-server/lldb-platform.cpp
@@ -22,7 +22,6 @@
 #include 
 
 #include "llvm/Support/FileSystem.h"
-#include "llvm/Support/FileUtilities.h"
 #include "llvm/Support/WithColor.h"
 #include "llvm/Support/raw_ostream.h"
 
@@ -103,38 +102,14 @@
 return Status("Failed to create directory %s: %s",
   temp_file_spec.GetPath().c_str(), error.AsCString());
 
-  llvm::SmallString<64> temp_file_path;
-  temp_file_spec.AppendPathComponent("port-file.%%");
-  temp_file_path = temp_file_spec.GetPath();
-
   Status status;
-  if (auto Err =
-  handleErrors(llvm::writeFileAtomically(
-   temp_file_path, file_spec.GetPath(), socket_id),
-   [, _spec](const AtomicFileWriteError ) {
- std::string ErrorMsgBuffer;
- llvm::raw_string_ostream S(ErrorMsgBuffer);
- E.log(S);
-
- switch (E.Error) {
- case atomic_write_error::failed_to_create_uniq_file:
-   status = Status("Failed to create temp file: %s",
-   ErrorMsgBuffer.c_str());
-   break;
- case atomic_write_error::output_stream_error:
-   status = Status("Failed to write to port file.");
-   break;
- case atomic_write_error::failed_to_rename_temp_file:
-   status = Status("Failed to rename file %s to %s: %s",
-   ErrorMsgBuffer.c_str(),
-   file_spec.GetPath().c_str(),
-   ErrorMsgBuffer.c_str());
-   break;
- }
-   })) {
+  if (auto Err = llvm::writeToOutput(file_spec.GetPath(),
+ [_id](llvm::raw_ostream ) {
+