Use appropriate print helpers for debug messages.
Signed-off-by: Gregory CLEMENT <[email protected]>
---
drivers/tty/n_gsm.c | 14 ++------------
1 file changed, 2 insertions(+), 12 deletions(-)
diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
index d77ed82a4840..67c8f8173023 100644
--- a/drivers/tty/n_gsm.c
+++ b/drivers/tty/n_gsm.c
@@ -504,18 +504,8 @@ static void gsm_print_packet(const char *hdr, int addr, int cr,
else
pr_cont("(F)");
- if (dlen) {
- int ct = 0;
- while (dlen--) {
- if (ct % 8 == 0) {
- pr_cont("\n");
- pr_debug(" ");
- }
- pr_cont("%02X ", *data++);
- ct++;
- }
- }
- pr_cont("\n");
+ if (dlen)
+ print_hex_dump_bytes("", DUMP_PREFIX_NONE, data, dlen);
}
--
2.26.2
On 12. 05. 20, 13:53, Gregory CLEMENT wrote:
> Use appropriate print helpers for debug messages.
>
> Signed-off-by: Gregory CLEMENT <[email protected]>
> ---
> drivers/tty/n_gsm.c | 14 ++------------
> 1 file changed, 2 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
> index d77ed82a4840..67c8f8173023 100644
> --- a/drivers/tty/n_gsm.c
> +++ b/drivers/tty/n_gsm.c
> @@ -504,18 +504,8 @@ static void gsm_print_packet(const char *hdr, int addr, int cr,
> else
> pr_cont("(F)");
>
> - if (dlen) {
> - int ct = 0;
> - while (dlen--) {
> - if (ct % 8 == 0) {
> - pr_cont("\n");
> - pr_debug(" ");
> - }
> - pr_cont("%02X ", *data++);
> - ct++;
> - }
> - }
> - pr_cont("\n");
> + if (dlen)
This test is superfluous. print_hex_dump_* won't write anything when
zero length is passed to it.
> + print_hex_dump_bytes("", DUMP_PREFIX_NONE, data, dlen);
> }
thanks,
--
js
suse labs
Hello Jiri,
> On 12. 05. 20, 13:53, Gregory CLEMENT wrote:
>> Use appropriate print helpers for debug messages.
>>
>> Signed-off-by: Gregory CLEMENT <[email protected]>
>> ---
>> drivers/tty/n_gsm.c | 14 ++------------
>> 1 file changed, 2 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
>> index d77ed82a4840..67c8f8173023 100644
>> --- a/drivers/tty/n_gsm.c
>> +++ b/drivers/tty/n_gsm.c
>> @@ -504,18 +504,8 @@ static void gsm_print_packet(const char *hdr, int addr, int cr,
>> else
>> pr_cont("(F)");
>>
>> - if (dlen) {
>> - int ct = 0;
>> - while (dlen--) {
>> - if (ct % 8 == 0) {
>> - pr_cont("\n");
>> - pr_debug(" ");
>> - }
>> - pr_cont("%02X ", *data++);
>> - ct++;
>> - }
>> - }
>> - pr_cont("\n");
>> + if (dlen)
>
> This test is superfluous. print_hex_dump_* won't write anything when
> zero length is passed to it.
As I will send a v3 due to the issue found on the last patch, I am also
going to fix this.
Gregory
>
>> + print_hex_dump_bytes("", DUMP_PREFIX_NONE, data, dlen);
>> }
>
> thanks,
> --
> js
> suse labs
--
Gregory Clement, Bootlin
Embedded Linux and Kernel engineering
http://bootlin.com
On Mon, May 18, 2020 at 09:33:57AM +0200, Gregory CLEMENT wrote:
> Hello Jiri,
>
> > On 12. 05. 20, 13:53, Gregory CLEMENT wrote:
> >> Use appropriate print helpers for debug messages.
> >>
> >> Signed-off-by: Gregory CLEMENT <[email protected]>
> >> ---
> >> drivers/tty/n_gsm.c | 14 ++------------
> >> 1 file changed, 2 insertions(+), 12 deletions(-)
> >>
> >> diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
> >> index d77ed82a4840..67c8f8173023 100644
> >> --- a/drivers/tty/n_gsm.c
> >> +++ b/drivers/tty/n_gsm.c
> >> @@ -504,18 +504,8 @@ static void gsm_print_packet(const char *hdr, int addr, int cr,
> >> else
> >> pr_cont("(F)");
> >>
> >> - if (dlen) {
> >> - int ct = 0;
> >> - while (dlen--) {
> >> - if (ct % 8 == 0) {
> >> - pr_cont("\n");
> >> - pr_debug(" ");
> >> - }
> >> - pr_cont("%02X ", *data++);
> >> - ct++;
> >> - }
> >> - }
> >> - pr_cont("\n");
> >> + if (dlen)
> >
> > This test is superfluous. print_hex_dump_* won't write anything when
> > zero length is passed to it.
>
> As I will send a v3 due to the issue found on the last patch, I am also
> going to fix this.
Ugh, as I already applied this series, should I just revert them all, or
are you going to send fix-ups on top of what I have applied instead?
thanks,
greg k-h
Hello Greg,
> On Mon, May 18, 2020 at 09:33:57AM +0200, Gregory CLEMENT wrote:
>> Hello Jiri,
>>
>> > On 12. 05. 20, 13:53, Gregory CLEMENT wrote:
>> >> Use appropriate print helpers for debug messages.
>> >>
>> >> Signed-off-by: Gregory CLEMENT <[email protected]>
>> >> ---
>> >> drivers/tty/n_gsm.c | 14 ++------------
>> >> 1 file changed, 2 insertions(+), 12 deletions(-)
>> >>
>> >> diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
>> >> index d77ed82a4840..67c8f8173023 100644
>> >> --- a/drivers/tty/n_gsm.c
>> >> +++ b/drivers/tty/n_gsm.c
>> >> @@ -504,18 +504,8 @@ static void gsm_print_packet(const char *hdr, int addr, int cr,
>> >> else
>> >> pr_cont("(F)");
>> >>
>> >> - if (dlen) {
>> >> - int ct = 0;
>> >> - while (dlen--) {
>> >> - if (ct % 8 == 0) {
>> >> - pr_cont("\n");
>> >> - pr_debug(" ");
>> >> - }
>> >> - pr_cont("%02X ", *data++);
>> >> - ct++;
>> >> - }
>> >> - }
>> >> - pr_cont("\n");
>> >> + if (dlen)
>> >
>> > This test is superfluous. print_hex_dump_* won't write anything when
>> > zero length is passed to it.
>>
>> As I will send a v3 due to the issue found on the last patch, I am also
>> going to fix this.
>
> Ugh, as I already applied this series, should I just revert them all, or
> are you going to send fix-ups on top of what I have applied instead?
I was about to send a new series, but I can just send fix-ups. It's up
to you.
Gregory
>
> thanks,
>
> greg k-h
--
Gregory Clement, Bootlin
Embedded Linux and Kernel engineering
http://bootlin.com
On Mon, May 18, 2020 at 09:44:52AM +0200, Gregory CLEMENT wrote:
> Hello Greg,
>
> > On Mon, May 18, 2020 at 09:33:57AM +0200, Gregory CLEMENT wrote:
> >> Hello Jiri,
> >>
> >> > On 12. 05. 20, 13:53, Gregory CLEMENT wrote:
> >> >> Use appropriate print helpers for debug messages.
> >> >>
> >> >> Signed-off-by: Gregory CLEMENT <[email protected]>
> >> >> ---
> >> >> drivers/tty/n_gsm.c | 14 ++------------
> >> >> 1 file changed, 2 insertions(+), 12 deletions(-)
> >> >>
> >> >> diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
> >> >> index d77ed82a4840..67c8f8173023 100644
> >> >> --- a/drivers/tty/n_gsm.c
> >> >> +++ b/drivers/tty/n_gsm.c
> >> >> @@ -504,18 +504,8 @@ static void gsm_print_packet(const char *hdr, int addr, int cr,
> >> >> else
> >> >> pr_cont("(F)");
> >> >>
> >> >> - if (dlen) {
> >> >> - int ct = 0;
> >> >> - while (dlen--) {
> >> >> - if (ct % 8 == 0) {
> >> >> - pr_cont("\n");
> >> >> - pr_debug(" ");
> >> >> - }
> >> >> - pr_cont("%02X ", *data++);
> >> >> - ct++;
> >> >> - }
> >> >> - }
> >> >> - pr_cont("\n");
> >> >> + if (dlen)
> >> >
> >> > This test is superfluous. print_hex_dump_* won't write anything when
> >> > zero length is passed to it.
> >>
> >> As I will send a v3 due to the issue found on the last patch, I am also
> >> going to fix this.
> >
> > Ugh, as I already applied this series, should I just revert them all, or
> > are you going to send fix-ups on top of what I have applied instead?
>
> I was about to send a new series, but I can just send fix-ups. It's up
> to you.
fix-ups are less work for me :)
Use a temporary buffer to avoid multiple pr_cont uses.
Signed-off-by: Joe Perches <[email protected]>
---
> > > Ugh, as I already applied this series, should I just revert them all, or
> > > are you going to send fix-ups on top of what I have applied instead?
> >
> > I was about to send a new series, but I can just send fix-ups. It's up
> > to you.
>
> fix-ups are less work for me :)
Perhaps use something like this instead?
It does increase object size a tiny bit.
drivers/tty/n_gsm.c | 71 +++++++++++++++++++++++++++++++----------------------
1 file changed, 42 insertions(+), 29 deletions(-)
diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
index 69200bd411f7..7d7820aeb57b 100644
--- a/drivers/tty/n_gsm.c
+++ b/drivers/tty/n_gsm.c
@@ -454,58 +454,71 @@ static u8 gsm_encode_modem(const struct gsm_dlci *dlci)
*/
static void gsm_print_packet(const char *hdr, int addr, int cr,
- u8 control, const u8 *data, int dlen)
+ u8 control, const u8 *data, int dlen)
{
+ char buf[100];
+ char *p;
+ char *type;
+
if (!(debug & 1))
return;
- pr_info("%s %d) %c: ", hdr, addr, "RC"[cr]);
+ p = buf;
+ p += scnprintf(p, buf - p, "%s %d) %c: ", hdr, addr, "RC"[cr]);
switch (control & ~PF) {
case SABM:
- pr_cont("SABM");
+ type = "SABM";
break;
case UA:
- pr_cont("UA");
+ type = "UA";
break;
case DISC:
- pr_cont("DISC");
+ type = "DISC";
break;
case DM:
- pr_cont("DM");
+ type = "DM";
break;
case UI:
- pr_cont("UI");
+ type = "UI";
break;
case UIH:
- pr_cont("UIH");
+ type = "UIH";
break;
default:
- if (!(control & 0x01)) {
- pr_cont("I N(S)%d N(R)%d",
- (control & 0x0E) >> 1, (control & 0xE0) >> 5);
- } else switch (control & 0x0F) {
- case RR:
- pr_cont("RR(%d)", (control & 0xE0) >> 5);
- break;
- case RNR:
- pr_cont("RNR(%d)", (control & 0xE0) >> 5);
- break;
- case REJ:
- pr_cont("REJ(%d)", (control & 0xE0) >> 5);
- break;
- default:
- pr_cont("[%02X]", control);
+ type = NULL;
+ break;
+ }
+
+ if (type) {
+ p += scnprintf(p, buf - p, "%s", type);
+ } else if (!(control & 0x01)) {
+ p += scnprintf(p, buf - p, "I N(S)%d N(R)%d",
+ (control & 0x0E) >> 1, (control & 0xE0) >> 5);
+ } else {
+ switch (control & 0x0F) {
+ case RR:
+ p += scnprintf(p, buf - p, "RR(%d)",
+ (control & 0xE0) >> 5);
+ break;
+ case RNR:
+ p += scnprintf(p, buf - p, "RNR(%d)",
+ (control & 0xE0) >> 5);
+ break;
+ case REJ:
+ p += scnprintf(p, buf - p, "REJ(%d)",
+ (control & 0xE0) >> 5);
+ break;
+ default:
+ p += scnprintf(p, buf - p, "[%02X]", control);
+ break;
}
}
- if (control & PF)
- pr_cont("(P)");
- else
- pr_cont("(F)");
+ p += scnprintf(p, buf - p, "(%c)", control & PF ? 'P' : 'F');
- if (dlen)
- print_hex_dump_bytes("", DUMP_PREFIX_NONE, data, dlen);
+ pr_info("%s\n", buf);
+ print_hex_dump_bytes("", DUMP_PREFIX_NONE, data, dlen);
}