Messages in this thread | | | Date | Fri, 3 Sep 2021 16:21:46 +0200 | From | Len Baker <> | Subject | Re: [PATCH v5] EDAC/mc: Prefer strscpy or scnprintf over strcpy |
| |
Hi,
On Sun, Aug 29, 2021 at 09:38:37AM -0700, Joe Perches wrote: > On Sun, 2021-08-29 at 18:15 +0200, Len Baker wrote: > > strcpy() performs no bounds checking on the destination buffer. This > > could result in linear overflows beyond the end of the buffer, leading > > to all kinds of misbehaviors. The safe replacement is strscpy() [1]. > > > > However, to simplify and clarify the code, to concatenate labels use > > the scnprintf() function. This way it is not necessary to check the > > return value of strscpy (-E2BIG if the parameter count is 0 or the src > > was truncated) since the scnprintf returns always the number of chars > > written into the buffer. This function returns always a nul-terminated > > string even if it needs to be truncated. > [] > > diff --git a/drivers/edac/edac_mc.c b/drivers/edac/edac_mc.c > [] > > @@ -1113,12 +1116,11 @@ void edac_mc_handle_error(const enum hw_event_mc_err_type type, > > p = e->label; > > *p = '\0'; > > } else { > > - if (p != e->label) { > > - strcpy(p, OTHER_LABEL); > > - p += strlen(OTHER_LABEL); > > - } > > - strcpy(p, dimm->label); > > - p += strlen(p); > > + const char *or = (p != e->label) ? OTHER_LABEL : ""; > > + > > + n = scnprintf(p, len, "%s%s", or, dimm->label); > > + len -= n; > > + p += n; > > A very common and intelligible mechanism for this is: > > const char *prefix = ""; > int n = 0; > ... > n += scnprintf(e->label + n, sizeof(e->label) - n, > "%s%s", prefix, dimm->label); > prefix = " or "; > Ok, I will send a new version using this common mechanism. Thanks for the advise.
Regards, Len
| |