Re: checkpatch minor issue

Joe Perches
2014-07-09T18:50:55Z

On Wed, 2014-07-09 at 16:00 +0100, Zoltan Kiss wrote:
> In one of my posted patch David spotted an indentation error, I thought
> these trivial mistakes are catched by checkpatch. Maybe it shouldn't,
> but otherwise you should check this issue, see it in the mail below.

Coverity or coccinelle might work better for this.

I think it's not possible to be found via checkpatch,
but maybe Andy has a better idea.

btw: Even if you applied the patch and checked the
patched file, checkpatch doesn't find it.
It does say:

ERROR: else should follow close brace '}'
#1145: FILE: net/core/pktgen.c:1145:
+ }
+ else

Andy, Here's the original patch: http://patchwork.ozlabs.org/patch/366468/
---
diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index 5b05e36..4465249 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -202,6 +202,7 @@
#define F_QUEUE_MAP_CPU (1<<14) /* queue map mirrors smp_processor_id() */
#define F_NODE (1<<15) /* Node memory alloc*/
#define F_UDPCSUM (1<<16) /* Include UDP checksum */
+#define F_PATTERN (1<<17) /* Fill the payload with a pattern */

/* Thread control flag bits */
#define T_STOP (1<<0) /* Stop run */
@@ -257,7 +258,7 @@ struct pktgen_dev {
int max_pkt_size;
int pkt_overhead; /* overhead for MPLS, VLANs, IPSEC etc */
int nfrags;
- struct page *page;
+ struct page *pages[MAX_SKB_FRAGS];
u64 delay; /* nano-seconds */

__u64 count; /* Default No packets to send */
@@ -638,6 +639,9 @@ static int pktgen_if_show(struct seq_file *seq, void *v)
if (pkt_dev->flags & F_UDPCSUM)
seq_puts(seq, "UDPCSUM ");

+ if (pkt_dev->flags & F_PATTERN)
+ seq_printf(seq, "PATTERN ");
+
if (pkt_dev->flags & F_MPLS_RND)
seq_puts(seq, "MPLS_RND ");

@@ -1129,11 +1133,13 @@ static ssize_t pktgen_if_write(struct file *file,
i += len;

if (node_possible(value)) {
+ int j;
pkt_dev->node = value;
sprintf(pg_result, "OK: node=%d", pkt_dev->node);
- if (pkt_dev->page) {
- put_page(pkt_dev->page);
- pkt_dev->page = NULL;
+ for (j = 0; j < MAX_SKB_FRAGS; ++j)
+ if (pkt_dev->pages[j]) {
+ put_page(pkt_dev->pages[j]);
+ pkt_dev->pages[j] = NULL;
}
}
else
@@ -1244,6 +1250,12 @@ static ssize_t pktgen_if_write(struct file *file,
else if (strcmp(f, "!UDPCSUM") == 0)
pkt_dev->flags &= ~F_UDPCSUM;

+ else if (strcmp(f, "PATTERN") == 0)
+ pkt_dev->flags |= F_PATTERN;
+
+ else if (strcmp(f, "!PATTERN") == 0)
+ pkt_dev->flags &= ~F_PATTERN;
+
else {
sprintf(pg_result,
"Flag -:%s:- unknown\nAvailable flags, (prepend ! to un-set flag):\n%s",
@@ -2625,17 +2637,98 @@ static inline __be16 build_tci(unsigned int id, unsigned int cfi,
return htons(id | (cfi << 12) | (prio << 13));
}

+/* Max number of digits. The sizeof equals to log base 2^8 (UINT_MAX), multiply
+ * with 3 is a cheap, rounded up conversion to log10
+ */
+#define UINT_MAX_DIGITS (3*sizeof(unsigned long)+1)
+
+/* Fill the buffer up with a pattern
+ * buf - pointer to buffer
+ * bufsize - size of the buffer
+ * start - starting value for the pattern
+ * incomplete - pointer to the offset inside the pattern, or 0 if none
+ *
+ * The pattern is a repetition of " %lu", a series of increasing numbers divided
+ * by space. The value of the number is "start" plus the size of the preceding
+ * space. E.g. if start is 1000, it starts like " 1000 1005 1010".
+ * If the last number doesn't fit, it gets truncated, and the number of leading
+ * characters is saved into incomplete. It can be passed then to the next call
+ * with the next buffer, and the pattern looks contiguous over scattered
+ * buffers.
+ * It returns "start" plus the offset of the byte after the last full
+ * pattern write.
+ */
+static unsigned long pattern_to_packet(char *buf,
+ int bufsize,
+ unsigned long start,
+ unsigned int *incomplete)
+{
+ int len;
+ /* Only used when the pattern doesn't align to the buffer */
+ char temp[UINT_MAX_DIGITS];
+
+ if (*incomplete) {
+ int copylen, offset = *incomplete;
+
+ len = snprintf(temp, sizeof(temp), "%lu ", start);
+ copylen = len - *incomplete;
+ if (copylen > bufsize) {
+ /* The continuation of this number couldn't fit here */
+ copylen = bufsize;
+ *incomplete += bufsize;
+ } else {
+ *incomplete = 0;
+ start += len;
+ }
+ memcpy(buf, temp + offset, copylen);
+ bufsize -= copylen;
+ buf += copylen;
+ }
+
+ while (bufsize > 0) {
+ len = snprintf(buf, bufsize, " %lu", start);
+ /* The last number doesn't fit, remember where it was truncated.
+ */
+ if (len >= bufsize) {
+ /* snprintf always add a trailing zero, but actually we
+ * need the last digit there
+ */
+ len = snprintf(temp, sizeof(temp), " %lu", start);
+ memcpy(buf, temp, bufsize);
+ /* If the last number just fit without the trailing
+ * zero, the next buffer can continue from an increased
+ * offset.
+ */
+ if (len == bufsize)
+ start += len;
+ *incomplete = bufsize;
+ return start;
+ }
+ bufsize -= len;
+ start += len;
+ buf += len;
+ }
+ return start;
+}
+
static void pktgen_finalize_skb(struct pktgen_dev *pkt_dev, struct sk_buff *skb,
int datalen)
{
struct timeval timestamp;
struct pktgen_hdr *pgh;
+ unsigned long offset = 0;
+ unsigned int incomplete = 0;

pgh = (struct pktgen_hdr *)skb_put(skb, sizeof(*pgh));
datalen -= sizeof(*pgh);

if (pkt_dev->nfrags <= 0) {
- memset(skb_put(skb, datalen), 0, datalen);
+ if (pkt_dev->flags & F_PATTERN)
+ offset = pattern_to_packet(skb_put(skb, datalen),
+ datalen, offset,
+ &incomplete);
+ else
+ memset(skb_put(skb, datalen), 0, datalen);
} else {
int frags = pkt_dev->nfrags;
int i, len;
@@ -2646,7 +2739,12 @@ static void pktgen_finalize_skb(struct pktgen_dev *pkt_dev, struct sk_buff *skb,
frags = MAX_SKB_FRAGS;
len = datalen - frags * PAGE_SIZE;
if (len > 0) {
- memset(skb_put(skb, len), 0, len);
+ if (pkt_dev->flags & F_PATTERN)
+ offset = pattern_to_packet(skb_put(skb, len),
+ len, offset,
+ &incomplete);
+ else
+ memset(skb_put(skb, len), 0, len);
datalen = frags * PAGE_SIZE;
}

@@ -2654,17 +2752,26 @@ static void pktgen_finalize_skb(struct pktgen_dev *pkt_dev, struct sk_buff *skb,
frag_len = (datalen/frags) < PAGE_SIZE ?
(datalen/frags) : PAGE_SIZE;
while (datalen > 0) {
- if (unlikely(!pkt_dev->page)) {
+ int fragpage;
+ gfp_t flags = GFP_KERNEL | __GFP_ZERO;
+
+ if (pkt_dev->flags & F_PATTERN)
+ fragpage = i;
+ else
+ fragpage = 0;
+
+ if (unlikely(!pkt_dev->pages[fragpage])) {
int node = numa_node_id();

if (pkt_dev->node >= 0 && (pkt_dev->flags & F_NODE))
node = pkt_dev->node;
- pkt_dev->page = alloc_pages_node(node, GFP_KERNEL | __GFP_ZERO, 0);
- if (!pkt_dev->page)
+ pkt_dev->pages[fragpage] = alloc_pages_node(node, flags, 0);
+ if (!pkt_dev->pages[fragpage])
break;
}
- get_page(pkt_dev->page);
- skb_frag_set_page(skb, i, pkt_dev->page);
+ get_page(pkt_dev->pages[fragpage]);
+ skb_frag_set_page(skb, i, pkt_dev->pages[fragpage]);
+
skb_shinfo(skb)->frags[i].page_offset = 0;
/*last fragment, fill rest of data*/
if (i == (frags - 1))
@@ -2673,6 +2780,10 @@ static void pktgen_finalize_skb(struct pktgen_dev *pkt_dev, struct sk_buff *skb,
else
skb_frag_size_set(&skb_shinfo(skb)->frags[i], frag_len);
datalen -= skb_frag_size(&skb_shinfo(skb)->frags[i]);
+ if (pkt_dev->flags & F_PATTERN)
+ offset = pattern_to_packet(skb_frag_address(&skb_shinfo(skb)->frags[i]),
+ skb_frag_size(&skb_shinfo(skb)->frags[i]),
+ offset, &incomplete);
skb->len += skb_frag_size(&skb_shinfo(skb)->frags[i]);
skb->data_len += skb_frag_size(&skb_shinfo(skb)->frags[i]);
i++;
@@ -3682,6 +3793,8 @@ static void _rem_dev_from_if_list(struct pktgen_thread *t,
static int pktgen_remove_device(struct pktgen_thread *t,
struct pktgen_dev *pkt_dev)
{
+ int i;
+
pr_debug("remove_device pkt_dev=%p\n", pkt_dev);

if (pkt_dev->running) {
@@ -3709,8 +3822,9 @@ static int pktgen_remove_device(struct pktgen_thread *t,
free_SAs(pkt_dev);
#endif
vfree(pkt_dev->flows);
- if (pkt_dev->page)
- put_page(pkt_dev->page);
+ for (i = 0; i < MAX_SKB_FRAGS; ++i)
+ if (pkt_dev->pages[i])
+ put_page(pkt_dev->pages[i]);
kfree_rcu(pkt_dev, rcu);
return 0;
}