lcm-proj / lcm

Lightweight Communications and Marshalling
GNU Lesser General Public License v2.1
944 stars 385 forks source link

Tweak previous strcpy changes #440

Closed tprk77 closed 1 year ago

tprk77 commented 1 year ago

For the changes to debug, there is no need to malloc there because 256 chars is more than enough to hold everything in DBG_NAMETAB. But if there is garbage in that variable, make sure to null terminate.

For mpudpm and udpm, there is no need to copy past the end of the string, so use strcpy for that. There is no need for strncpy because the size is already checked in those functions.

ihilt commented 1 year ago

For the changes to debug, there is no need to malloc there because 256 chars is more than enough to hold everything in DBG_NAMETAB. But if there is garbage in that variable, make sure to null terminate.

That's sensible. I'm not sure how possible it is in the future for LCM_DBG to expand but terminating env with '\0' will prevent overflow.

For mpudpm and udpm, there is no need to copy past the end of the string, so use strcpy for that. There is no need for strncpy because the size is already checked in those functions.

Since we've already determined the length with strlen, we could avoid the operation of checking for '\0' in strcpy and use memcpy instead.

diff --git a/lcm/lcm_udpm.c b/lcm/lcm_udpm.c
index ec0f03c..d91118b 100644
--- a/lcm/lcm_udpm.c
+++ b/lcm/lcm_udpm.c
@@ -264,10 +264,10 @@ static int _recv_message_fragment(lcm_udpm_t *lcm, lcm_buf_t *lcmb, uint32_t sz)

     // if this is the first packet, set some values
     char *channel = NULL;
+    int channel_sz = 0;
     if (hdr->fragment_no == 0) {
         channel = (char *) (hdr + 1);
-        char *channel = (char *) (hdr + 1);
-        int channel_sz = strlen(channel);
+        channel_sz = strlen(channel);
         if (channel_sz > LCM_MAX_CHANNEL_NAME_LENGTH) {
             dbg(DBG_LCM, "bad channel name length\n");
             lcm->udp_discarded_bad++;
@@ -284,8 +284,7 @@ static int _recv_message_fragment(lcm_udpm_t *lcm, lcm_buf_t *lcmb, uint32_t sz)
     }

     if (channel != NULL) {
-        // The channel name length is already checked above
-        strcpy(fbuf->channel, channel);
+        memcpy(fbuf->channel, channel, channel_sz + 1);
     }

 #ifdef __linux__
tprk77 commented 1 year ago

Good idea, I will make that change.