| Message ID | 20230620083411.508797-3-wintera@linux.ibm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | s390/net: updates 2023-06-10 | expand |
On Tue, Jun 20, 2023 at 10:34:09AM +0200, Alexandra Winter wrote: > From: Thorsten Winkler <twinkler@linux.ibm.com> > > This LWN article explains the rationale for this change > https: //lwn.net/Articles/69419/ > Ie. snprintf() returns what *would* be the resulting length, > while scnprintf() returns the actual length. Hi Alexandra, Although I agree that it's nice to use scnprintf() the justification given seems a bit odd: it talks of the return value but it is ignored both before and after this patch. Likewise for some of the changes in patch 4/4. Also is it intentional that there is a space in the URL immediately after 'http:' ? Maybe mangled by something. Not that it really maters AFAIC. > Reported-by: Jules Irenge <jbi.octave@gmail.com> > Reviewed-by: Alexandra Winter <wintera@linux.ibm.com> > Signed-off-by: Thorsten Winkler <twinkler@linux.ibm.com> > Signed-off-by: Alexandra Winter <wintera@linux.ibm.com> > --- > drivers/s390/net/lcs.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/s390/net/lcs.h b/drivers/s390/net/lcs.h > index bd52caa3b11b..a2699b70b050 100644 > --- a/drivers/s390/net/lcs.h > +++ b/drivers/s390/net/lcs.h > @@ -21,7 +21,7 @@ do { \ > #define LCS_DBF_TEXT_(level,name,text...) \ > do { \ > if (debug_level_enabled(lcs_dbf_##name, level)) { \ > - sprintf(debug_buffer, text); \ > + scnprintf(debug_buffer, sizeof(debug_buffer), text); \ > debug_text_event(lcs_dbf_##name, level, debug_buffer); \ > } \ > } while (0) > -- > 2.39.2 > >
On 20.06.23 21:16, Simon Horman wrote: > On Tue, Jun 20, 2023 at 10:34:09AM +0200, Alexandra Winter wrote: >> From: Thorsten Winkler <twinkler@linux.ibm.com> >> >> This LWN article explains the rationale for this change >> https: //lwn.net/Articles/69419/ >> Ie. snprintf() returns what *would* be the resulting length, >> while scnprintf() returns the actual length. > Hi Alexandra, > > Although I agree that it's nice to use scnprintf() the justification given > seems a bit odd: it talks of the return value but it is ignored both before > and after this patch. > > Likewise for some of the changes in patch 4/4. You are correct. The main improvement of these patches is to get rid of sprintf. And we decided to use scnprintf everywhere. I'll send a v2 with a slightly updated description. > > Also is it intentional that there is a space in the URL immediately > after 'http:' ? Maybe mangled by something. Not that it really maters > AFAIC. Thanks for spotting this, Simon. Corrected in v2.
diff --git a/drivers/s390/net/lcs.h b/drivers/s390/net/lcs.h index bd52caa3b11b..a2699b70b050 100644 --- a/drivers/s390/net/lcs.h +++ b/drivers/s390/net/lcs.h @@ -21,7 +21,7 @@ do { \ #define LCS_DBF_TEXT_(level,name,text...) \ do { \ if (debug_level_enabled(lcs_dbf_##name, level)) { \ - sprintf(debug_buffer, text); \ + scnprintf(debug_buffer, sizeof(debug_buffer), text); \ debug_text_event(lcs_dbf_##name, level, debug_buffer); \ } \ } while (0)