[GitHub] [zookeeper] phunt commented on a change in pull request #846: ZOOKEEPER-3302 ZooKeeper C client does not compile on Fedora 29

[GitBox <gi...@apache.org>]

phunt commented on a change in pull request #846: ZOOKEEPER-3302 ZooKeeper C client does not compile on Fedora 29
URL: https://github.com/apache/zookeeper/pull/846#discussion_r264010745
 
 

 ##########
 File path: zookeeper-client/zookeeper-client-c/src/cli.c
 ##########
 @@ -686,7 +686,7 @@ int main(int argc, char **argv) {
                   sizeof(cmd));
           return 2;
         }
-        strncpy(cmd, argv[2]+4, sizeof(cmd));
+        strncpy(cmd, argv[2]+4, sizeof(cmd) - 1);
 
 Review comment:
   This seems a bit of a bogus "fix" specifically for the truncation warning. The change is basically saying that the src n is less than the max buffer size. This happens to be true here given the "cmdlen" check above, and the "+4" offset on the src. However it's a bit of a hash, no?
   
   A google for "memcpy vs strncpy" shows similar debates, and I also see google results for "stringop-truncation" which results in links such as https://stackoverflow.com/questions/50198319/gcc-8-wstringop-truncation-what-is-the-good-practice
   
   @eolivelli should we consider changing the idiom here? What do you think?

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services