szaszm commented on a change in pull request #731: MINIFICPP-1096 fix 
BackTrace, OOB indexing, tests, appveyor reporting
URL: https://github.com/apache/nifi-minifi-cpp/pull/731#discussion_r385805427
 
 

 ##########
 File path: libminifi/include/utils/file/FileUtils.h
 ##########
 @@ -112,31 +112,28 @@ class FileUtils {
 #endif
   }
 
-  static std::string create_temp_directory(char *format) {
+  static std::string create_temp_directory(char * const format) {
 
 Review comment:
   Pointers are objects. My interpretation of the exception is that the issue 
is too minor to be enforced by static analyzers, and is commonly not followed, 
hence the "false positives". In other words, my interpretation of the exception 
doesn't suggest that function argument objects shouldn't be declared `const`.
   
   I can see the point behind the readability argument, since `const` is 
present in the argument list, yet, the string behind the pointer is mutated. I 
think the risk of misunderstanding is minor given that we expect people who 
view the source to know the language well enough to know what `const` applies 
to. With this assumption, IMO the value gained by extra static analysis is 
greater than the value of lost readability.
   
   I couldn't find any clear guidelines on the `const`-ness of argument 
pointers. I'm willing to remove `const` if you insist on it.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[email protected]


With regards,
Apache Git Services

Reply via email to