Messages in this thread | | | From | NeilBrown <> | Date | Fri, 27 Mar 2020 08:44:17 +1100 | Subject | Re: [PATCH 2/2] SUNRPC: Optimize 'svc_print_xprts()' |
| |
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. That last version is a change of behaviour in that it will skip over lines that are too long, rather than aborting on the first one. I don't know which is preferred.
Thanks, NeilBrown
> > And Chuck Lever confirmed that: > That's exactly what this function is trying to avoid. As part of any > change in this area, it would be good to replace the current block > comment before this function with a Doxygen-format comment that > documents that goal. > > So, I will only send a V2 based on comments already received. > > CJ [unhandled content-type:application/pgp-signature] | |