jdanekrh commented on issue #176: NO-JIRA: [c] Fix Coverity warning of buffer 
overrun in pn_proactor_addr
URL: https://github.com/apache/qpid-proton/pull/176#issuecomment-465889752
 
 
   > Also doing strncat(host); strncat(':'); strncat(port) is wrong. The output 
buffer should start with strncpy() as we don't want to append to anything 
already in the buffer but overwrite it (as documented in the header file)
   
   `strncat` should be perfectly fine. First thing the code does (both Alan's 
version and mine) is `buf[0] = '\0';`. strncat will be then overwriting 
whatever was in buf, because it sees it as empty string to start with.
   
   > How about something like (compiles and passes tests with valgrind, but not 
tested with sanitisers or coverity):
   
   Looks reasonable, and does not require new function...
   
   `if (port) strncat(buf, port, len-hostlen-1);` I am thinking maybe `-2`, 
because strncat always adds `\0`. I'd need to run this in valgrind myself...
   
   I don't know (yet) if proton has tests that exercise this logic, or if all 
hostnames are relatively short compared to size of that buffer.
   
   

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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org

Reply via email to