From: Daniel Borkmann <dborkman@redhat.com>
To: Joe Perches <joe@perches.com>
Cc: davem@davemloft.net, netdev@vger.kernel.org, linux-sctp@vger.kernel.org
Subject: Re: [PATCH net-next v4 2/2] net: sctp: rework debugging framework to use pr_debug and friends
Date: Fri, 28 Jun 2013 22:40:30 +0000 [thread overview]
Message-ID: <51CE10DE.20006@redhat.com> (raw)
In-Reply-To: <1372458077.29380.30.camel@joe-AO722>
On 06/29/2013 12:21 AM, Joe Perches wrote:
> On Fri, 2013-06-28 at 19:49 +0200, Daniel Borkmann wrote:
>
>> @@ -536,11 +540,8 @@ void sctp_assoc_rm_peer(struct sctp_association *asoc,
>> struct list_head *pos;
>> struct sctp_transport *transport;
>>
>> - SCTP_DEBUG_PRINTK_IPADDR("sctp_assoc_rm_peer:association %p addr: ",
>> - " port: %d\n",
>> - asoc,
>> - (&peer->ipaddr),
>> - ntohs(peer->ipaddr.v4.sin_port));
>> + pr_debug("%s: association:%p addr:%pISpc\n",
>> + __func__, asoc, &peer->ipaddr.sa);
>
> Hi again Daniel.
>
> Perhaps the format is better written as "%pIScp"
> with the port after the compression as that's how
> the actual output would be produced.
Actually it really doesn't matter. One could also argue that this way it's
from generic to specific ... i.e. "%pISp" is the generic part as it's valid
for both IPv4/IPv6, and "c" is a IPv6-specific appendix.
>> @@ -636,12 +637,8 @@ struct sctp_transport *sctp_assoc_add_peer(struct sctp_association *asoc,
> []
>> + pr_debug("%s: association:%p addr:%pISpc state:%d\n", __func__,
>> + asoc, &addr->sa, peer_state);
>
> etc...
>
>> + pr_debug("%s: sending window update SACK- asoc:%p rwnd:%u "
>> + "a_rwnd:%u\n", __func__, asoc, asoc->rwnd,
>
> Please coalesce formats. It doesn't matter if
> the coalesced format exceeds 80 cols.
>
> pr_debug("%s: sending window update SACK - asoc:%p rwnd:%u a_rwnd:%u\n",
> __func__, asoc, asoc->rwnd, asoc->a_rwnd);
>
> btw: what I try to do with formats and args
> is to use a single line when it fits 80 cols
> and otherwise put all arguments after the
> format on a separate line.
Hm, as this is something cosmetic, really really minor and does not concern
all pr_debugs, are you okay if I "fix" some of them (where appropriate) as
a follow-up? Thanks !
WARNING: multiple messages have this Message-ID (diff)
From: Daniel Borkmann <dborkman@redhat.com>
To: Joe Perches <joe@perches.com>
Cc: davem@davemloft.net, netdev@vger.kernel.org, linux-sctp@vger.kernel.org
Subject: Re: [PATCH net-next v4 2/2] net: sctp: rework debugging framework to use pr_debug and friends
Date: Sat, 29 Jun 2013 00:40:30 +0200 [thread overview]
Message-ID: <51CE10DE.20006@redhat.com> (raw)
In-Reply-To: <1372458077.29380.30.camel@joe-AO722>
On 06/29/2013 12:21 AM, Joe Perches wrote:
> On Fri, 2013-06-28 at 19:49 +0200, Daniel Borkmann wrote:
>
>> @@ -536,11 +540,8 @@ void sctp_assoc_rm_peer(struct sctp_association *asoc,
>> struct list_head *pos;
>> struct sctp_transport *transport;
>>
>> - SCTP_DEBUG_PRINTK_IPADDR("sctp_assoc_rm_peer:association %p addr: ",
>> - " port: %d\n",
>> - asoc,
>> - (&peer->ipaddr),
>> - ntohs(peer->ipaddr.v4.sin_port));
>> + pr_debug("%s: association:%p addr:%pISpc\n",
>> + __func__, asoc, &peer->ipaddr.sa);
>
> Hi again Daniel.
>
> Perhaps the format is better written as "%pIScp"
> with the port after the compression as that's how
> the actual output would be produced.
Actually it really doesn't matter. One could also argue that this way it's
from generic to specific ... i.e. "%pISp" is the generic part as it's valid
for both IPv4/IPv6, and "c" is a IPv6-specific appendix.
>> @@ -636,12 +637,8 @@ struct sctp_transport *sctp_assoc_add_peer(struct sctp_association *asoc,
> []
>> + pr_debug("%s: association:%p addr:%pISpc state:%d\n", __func__,
>> + asoc, &addr->sa, peer_state);
>
> etc...
>
>> + pr_debug("%s: sending window update SACK- asoc:%p rwnd:%u "
>> + "a_rwnd:%u\n", __func__, asoc, asoc->rwnd,
>
> Please coalesce formats. It doesn't matter if
> the coalesced format exceeds 80 cols.
>
> pr_debug("%s: sending window update SACK - asoc:%p rwnd:%u a_rwnd:%u\n",
> __func__, asoc, asoc->rwnd, asoc->a_rwnd);
>
> btw: what I try to do with formats and args
> is to use a single line when it fits 80 cols
> and otherwise put all arguments after the
> format on a separate line.
Hm, as this is something cosmetic, really really minor and does not concern
all pr_debugs, are you okay if I "fix" some of them (where appropriate) as
a follow-up? Thanks !
next prev parent reply other threads:[~2013-06-28 22:40 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-06-28 17:49 [PATCH net-next v4 0/2] Rework SCTP debugging framework Daniel Borkmann
2013-06-28 17:49 ` Daniel Borkmann
2013-06-28 17:49 ` [PATCH net-next v4 1/2] lib: vsprintf: add IPv4/v6 generic %p[Ii]S[pfs] format specifier Daniel Borkmann
2013-06-28 17:49 ` Daniel Borkmann
2013-06-28 22:08 ` Joe Perches
2013-06-28 22:08 ` Joe Perches
2013-06-28 17:49 ` [PATCH net-next v4 2/2] net: sctp: rework debugging framework to use pr_debug and friends Daniel Borkmann
2013-06-28 17:49 ` Daniel Borkmann
2013-06-28 22:21 ` Joe Perches
2013-06-28 22:21 ` Joe Perches
2013-06-28 22:40 ` Daniel Borkmann [this message]
2013-06-28 22:40 ` Daniel Borkmann
2013-06-28 22:44 ` Joe Perches
2013-06-28 22:44 ` Joe Perches
2013-07-02 6:22 ` [PATCH net-next v4 0/2] Rework SCTP debugging framework David Miller
2013-07-02 6:22 ` David Miller
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=51CE10DE.20006@redhat.com \
--to=dborkman@redhat.com \
--cc=davem@davemloft.net \
--cc=joe@perches.com \
--cc=linux-sctp@vger.kernel.org \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.