Re: [PATCH 5/6] ath6kl: remove a warning on a macro

[Joe Perches <joe@perches.com>]

On Sat, 2014-03-08 at 23:20 -0800, Joe Perches wrote:
> On Sun, 2014-03-09 at 09:10 +0200, Kalle Valo wrote:
> > Hi Joe,
> 
> Hi Kalle.
> 
> > I would need help with this checkpatch warning:
> 
> No idea what the warning is.
> 
> > > --- a/drivers/net/wireless/ath/ath6kl/debug.c
> > > +++ b/drivers/net/wireless/ath/ath6kl/debug.c
> > > @@ -798,12 +798,10 @@ static ssize_t ath6kl_endpoint_stats_read(struct file *file,
> > >  		return -ENOMEM;
> > >  
> > >  #define EPSTAT(name)							\
> > > -	do {								\
> > > -		len = print_endpoint_stat(target, buf, buf_len, len,	\
> > > -					  offsetof(struct htc_endpoint_stats, \
> > > -						   name),		\
> > > -					  #name);			\
> > > -	} while (0)
> > > +	(len = print_endpoint_stat(target, buf, buf_len, len,		\
> > > +				   offsetof(struct htc_endpoint_stats,	\
> > > +					    name),			\
> > > +				   #name))
> > 
> > I wasn't quite able to figure out what is the preferred style here. I
> > don't see how the () style is any better, but checkpatch didn't complain
> > at least.
> 
> No idea what the preferred style is, but
> I'd probably change the #define to
> 
> #define EPSTAT(name)							\
> 	print_endpoint_stat(target, buf, buf_len, len,			\
> 			    offsetof(struct htc_endpoint_stats, name),	\
> 			    #name)
> 
> and the uses to
> 
> 	len = EPSTAT(whatever);

Or maybe this:

 drivers/net/wireless/ath/ath6kl/debug.c | 62 ++++++++++++++++-----------------
 1 file changed, 30 insertions(+), 32 deletions(-)

diff --git a/drivers/net/wireless/ath/ath6kl/debug.c b/drivers/net/wireless/ath/ath6kl/debug.c
index dbfd17d..7a6b476 100644
--- a/drivers/net/wireless/ath/ath6kl/debug.c
+++ b/drivers/net/wireless/ath/ath6kl/debug.c
@@ -771,16 +771,17 @@ static unsigned int print_endpoint_stat(struct htc_target *target, char *buf,
 	int i;
 	struct htc_endpoint_stats *ep_st;
 	u32 *counter;
+	unsigned int nlen = len;
 
-	len += scnprintf(buf + len, buf_len - len, "%s:", name);
+	nlen += scnprintf(buf + nlen, buf_len - nlen, "%s:", name);
 	for (i = 0; i < ENDPOINT_MAX; i++) {
 		ep_st = &target->endpoint[i].ep_st;
 		counter = ((u32 *) ep_st) + (offset / 4);
-		len += scnprintf(buf + len, buf_len - len, " %u", *counter);
+		nlen += scnprintf(buf + nlen, buf_len - nlen, " %u", *counter);
 	}
-	len += scnprintf(buf + len, buf_len - len, "\n");
+	nlen += scnprintf(buf + nlen, buf_len - nlen, "\n");
 
-	return len;
+	return nlen - len;
 }
 
 static ssize_t ath6kl_endpoint_stats_read(struct file *file,
@@ -800,34 +801,31 @@ static ssize_t ath6kl_endpoint_stats_read(struct file *file,
 		return -ENOMEM;
 
 #define EPSTAT(name)							\
-	do {								\
-		len = print_endpoint_stat(target, buf, buf_len, len,	\
-					  offsetof(struct htc_endpoint_stats, \
-						   name),		\
-					  #name);			\
-	} while (0)
-
-	EPSTAT(cred_low_indicate);
-	EPSTAT(tx_issued);
-	EPSTAT(tx_pkt_bundled);
-	EPSTAT(tx_bundles);
-	EPSTAT(tx_dropped);
-	EPSTAT(tx_cred_rpt);
-	EPSTAT(cred_rpt_from_rx);
-	EPSTAT(cred_rpt_from_other);
-	EPSTAT(cred_rpt_ep0);
-	EPSTAT(cred_from_rx);
-	EPSTAT(cred_from_other);
-	EPSTAT(cred_from_ep0);
-	EPSTAT(cred_cosumd);
-	EPSTAT(cred_retnd);
-	EPSTAT(rx_pkts);
-	EPSTAT(rx_lkahds);
-	EPSTAT(rx_bundl);
-	EPSTAT(rx_bundle_lkahd);
-	EPSTAT(rx_bundle_from_hdr);
-	EPSTAT(rx_alloc_thresh_hit);
-	EPSTAT(rxalloc_thresh_byte);
+	print_endpoint_stat(target, buf, buf_len, len,			\
+			    offsetof(struct htc_endpoint_stats, name),	\
+			    #name)
+
+	len += EPSTAT(cred_low_indicate);
+	len += EPSTAT(tx_issued);
+	len += EPSTAT(tx_pkt_bundled);
+	len += EPSTAT(tx_bundles);
+	len += EPSTAT(tx_dropped);
+	len += EPSTAT(tx_cred_rpt);
+	len += EPSTAT(cred_rpt_from_rx);
+	len += EPSTAT(cred_rpt_from_other);
+	len += EPSTAT(cred_rpt_ep0);
+	len += EPSTAT(cred_from_rx);
+	len += EPSTAT(cred_from_other);
+	len += EPSTAT(cred_from_ep0);
+	len += EPSTAT(cred_cosumd);
+	len += EPSTAT(cred_retnd);
+	len += EPSTAT(rx_pkts);
+	len += EPSTAT(rx_lkahds);
+	len += EPSTAT(rx_bundl);
+	len += EPSTAT(rx_bundle_lkahd);
+	len += EPSTAT(rx_bundle_from_hdr);
+	len += EPSTAT(rx_alloc_thresh_hit);
+	len += EPSTAT(rxalloc_thresh_byte);
 #undef EPSTAT
 
 	if (len > buf_len)