> On Mar 26, 2020, at 5:44 PM, NeilBrown <neilb@xxxxxxx> wrote: > > On Thu, Mar 26 2020, Christophe JAILLET wrote: > >> Le 25/03/2020 à 23:53, NeilBrown a écrit : >>> Can I suggest something more like this: >>> diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c >>> index de3c077733a7..0292f45b70f6 100644 >>> --- a/net/sunrpc/svc_xprt.c >>> +++ b/net/sunrpc/svc_xprt.c >>> @@ -115,16 +115,9 @@ int svc_print_xprts(char *buf, int maxlen) >>> buf[0] = '\0'; >>> >>> spin_lock(&svc_xprt_class_lock); >>> - list_for_each_entry(xcl, &svc_xprt_class_list, xcl_list) { >>> - int slen; >>> - >>> - sprintf(tmpstr, "%s %d\n", xcl->xcl_name, xcl->xcl_max_payload); >>> - slen = strlen(tmpstr); >>> - if (len + slen > maxlen) >>> - break; >>> - len += slen; >>> - strcat(buf, tmpstr); >>> - } >>> + list_for_each_entry(xcl, &svc_xprt_class_list, xcl_list) >>> + len += scnprintf(buf + len, maxlen - len, "%s %d\n", >>> + xcl->xcl_name, xcl->xcl_max_payload); >>> spin_unlock(&svc_xprt_class_lock); >>> >>> return len; >>> >>> NeilBrown >> >> Hi, >> >> this was what I suggested in the patch: >> --- >> This patch should have no functional change. >> We could go further, use scnprintf and write directly in the >> destination >> buffer. However, this could lead to a truncated last line. >> --- > > Sorry - I missed that. > So add > > end = strrchr(tmpstr, '\n'); > if (end) > end[1] = 0; > else > tmpstr[0] = 0; > > or maybe something like > list_for_each_entry(xcl, &svc_xprt_class_list, xcl_list) { > int l = snprintf(buf + len, maxlen - len, "%s %d\n", > xcl->xcl_name, xcl->xcl_max_payload); > if (l < maxlen - len) > len += l; > } > buf[len] = 0; > > There really is no need to have the secondary buffer, and I think doing > so just complicates the code. In the interest of getting this fix into the upcoming merge window, let's stick with the temporary buffer approach. Thanks! -- Chuck Lever