mbox series

[0/8] examples/l2fwd: fix checkpatch reported issues

Message ID 1538995285-2040-1-git-send-email-anoob.joseph@caviumnetworks.com (mailing list archive)
Headers
Series examples/l2fwd: fix checkpatch reported issues |

Message

Anoob Joseph Oct. 8, 2018, 10:41 a.m. UTC 
  This patchset fixes multiple issues reported by checkpatch in l2fwd
code base. These issues would be flagged for any new copy of the file
and hence, fixing at the source.

Anoob Joseph (8):
  examples/l2fwd: remove quoted white space before newline
  examples/l2fwd: need space between two args
  examples/l2fwd: else should follow close brace
  examples/l2fwd: replace bare usage of 'unsigned'
  examples/l2fwd: follow convention for block comments
  examples/l2fwd: limit line to 80 char
  examples/l2fwd: space required between elements
  examples/l2fwd: remove null initialisation

 examples/l2fwd/main.c | 49 ++++++++++++++++++++++++++-----------------------
 1 file changed, 26 insertions(+), 23 deletions(-)

Comments

Ferruh Yigit Oct. 8, 2018, 11 a.m. UTC | #1 
On 10/8/2018 11:41 AM, Anoob Joseph wrote:
> This patchset fixes multiple issues reported by checkpatch in l2fwd
> code base. These issues would be flagged for any new copy of the file
> and hence, fixing at the source.
> 
> Anoob Joseph (8):
>   examples/l2fwd: remove quoted white space before newline
>   examples/l2fwd: need space between two args
>   examples/l2fwd: else should follow close brace
>   examples/l2fwd: replace bare usage of 'unsigned'
>   examples/l2fwd: follow convention for block comments
>   examples/l2fwd: limit line to 80 char
>   examples/l2fwd: space required between elements
>   examples/l2fwd: remove null initialisation

Hi Anoob,

I am not sure if it is good idea to get syntax only fixes, I would prefer to get
syntax fixes when some other code touches that area.

Thanks,
ferruh
Anoob Joseph Oct. 8, 2018, 11:29 a.m. UTC | #2 
Hi Ferruh,

On 08-10-2018 16:30, Ferruh Yigit wrote:
> External Email
>
> On 10/8/2018 11:41 AM, Anoob Joseph wrote:
>> This patchset fixes multiple issues reported by checkpatch in l2fwd
>> code base. These issues would be flagged for any new copy of the file
>> and hence, fixing at the source.
>>
>> Anoob Joseph (8):
>>    examples/l2fwd: remove quoted white space before newline
>>    examples/l2fwd: need space between two args
>>    examples/l2fwd: else should follow close brace
>>    examples/l2fwd: replace bare usage of 'unsigned'
>>    examples/l2fwd: follow convention for block comments
>>    examples/l2fwd: limit line to 80 char
>>    examples/l2fwd: space required between elements
>>    examples/l2fwd: remove null initialisation
> Hi Anoob,
>
> I am not sure if it is good idea to get syntax only fixes, I would prefer to get
> syntax fixes when some other code touches that area.
I'm preparing a new copy of l2fwd with support for eventmode(as 
suggested in, 
https://mails.dpdk.org/archives/dev/2018-August/109717.html). All these 
issues were flagged when I did the copy. What would be the better 
approach in that case? Fix it in just the new application or fix the 
source? Fixing in just the new app would mean, the two versions will 
have a diff at the time of copy. And next time someone touches these 
lines, the same problem would be there.

Thanks,
Anoob
Ferruh Yigit Oct. 8, 2018, 12:35 p.m. UTC | #3 
On 10/8/2018 12:29 PM, Joseph, Anoob wrote:
> Hi Ferruh,
> 
> On 08-10-2018 16:30, Ferruh Yigit wrote:
>> External Email
>>
>> On 10/8/2018 11:41 AM, Anoob Joseph wrote:
>>> This patchset fixes multiple issues reported by checkpatch in l2fwd
>>> code base. These issues would be flagged for any new copy of the file
>>> and hence, fixing at the source.
>>>
>>> Anoob Joseph (8):
>>>    examples/l2fwd: remove quoted white space before newline
>>>    examples/l2fwd: need space between two args
>>>    examples/l2fwd: else should follow close brace
>>>    examples/l2fwd: replace bare usage of 'unsigned'
>>>    examples/l2fwd: follow convention for block comments
>>>    examples/l2fwd: limit line to 80 char
>>>    examples/l2fwd: space required between elements
>>>    examples/l2fwd: remove null initialisation
>> Hi Anoob,
>>
>> I am not sure if it is good idea to get syntax only fixes, I would prefer to get
>> syntax fixes when some other code touches that area.
> I'm preparing a new copy of l2fwd with support for eventmode(as 
> suggested in, 
> https://mails.dpdk.org/archives/dev/2018-August/109717.html). All these 
> issues were flagged when I did the copy. What would be the better 
> approach in that case? Fix it in just the new application or fix the 
> source? Fixing in just the new app would mean, the two versions will 
> have a diff at the time of copy.

I see, make sense to not create syntax diff copied and original versions, I
missed the new copy part. So OK for this patch.
Thomas Monjalon Oct. 29, 2018, 2:19 a.m. UTC | #4 
08/10/2018 14:35, Ferruh Yigit:
> On 10/8/2018 12:29 PM, Joseph, Anoob wrote:
> > On 08-10-2018 16:30, Ferruh Yigit wrote:
> >> On 10/8/2018 11:41 AM, Anoob Joseph wrote:
> >>> This patchset fixes multiple issues reported by checkpatch in l2fwd
> >>> code base. These issues would be flagged for any new copy of the file
> >>> and hence, fixing at the source.
> >>>
> >>> Anoob Joseph (8):
> >>>    examples/l2fwd: remove quoted white space before newline
> >>>    examples/l2fwd: need space between two args
> >>>    examples/l2fwd: else should follow close brace
> >>>    examples/l2fwd: replace bare usage of 'unsigned'
> >>>    examples/l2fwd: follow convention for block comments
> >>>    examples/l2fwd: limit line to 80 char
> >>>    examples/l2fwd: space required between elements
> >>>    examples/l2fwd: remove null initialisation
> >> Hi Anoob,
> >>
> >> I am not sure if it is good idea to get syntax only fixes, I would prefer to get
> >> syntax fixes when some other code touches that area.
> > I'm preparing a new copy of l2fwd with support for eventmode(as 
> > suggested in, 
> > https://mails.dpdk.org/archives/dev/2018-August/109717.html). All these 
> > issues were flagged when I did the copy. What would be the better 
> > approach in that case? Fix it in just the new application or fix the 
> > source? Fixing in just the new app would mean, the two versions will 
> > have a diff at the time of copy.
> 
> I see, make sense to not create syntax diff copied and original versions, I
> missed the new copy part. So OK for this patch.

It was said that there will be not so much common code.
So why bothering to reformat the original example?

Anyway, if such cleanup is worth before duplicating,
please insert it in the same patchset as the new example.
Joseph, Anoob Oct. 29, 2018, 4:48 a.m. UTC | #5 
Hi Thomas,

> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: 29 October 2018 07:49
> To: Joseph, Anoob <Anoob.Joseph@cavium.com>
> Cc: dev@dpdk.org; Ferruh Yigit <ferruh.yigit@intel.com>; Bruce Richardson
> <bruce.richardson@intel.com>; Pablo de Lara
> <pablo.de.lara.guarch@intel.com>; Jacob, Jerin
> <Jerin.JacobKollanukkaran@cavium.com>; Athreya, Narayana Prasad
> <NarayanaPrasad.Athreya@cavium.com>
> Subject: Re: [dpdk-dev] [PATCH 0/8] examples/l2fwd: fix checkpatch reported
> issues
> 
> External Email
> 
> 08/10/2018 14:35, Ferruh Yigit:
> > On 10/8/2018 12:29 PM, Joseph, Anoob wrote:
> > > On 08-10-2018 16:30, Ferruh Yigit wrote:
> > >> On 10/8/2018 11:41 AM, Anoob Joseph wrote:
> > >>> This patchset fixes multiple issues reported by checkpatch in
> > >>> l2fwd code base. These issues would be flagged for any new copy of
> > >>> the file and hence, fixing at the source.
> > >>>
> > >>> Anoob Joseph (8):
> > >>>    examples/l2fwd: remove quoted white space before newline
> > >>>    examples/l2fwd: need space between two args
> > >>>    examples/l2fwd: else should follow close brace
> > >>>    examples/l2fwd: replace bare usage of 'unsigned'
> > >>>    examples/l2fwd: follow convention for block comments
> > >>>    examples/l2fwd: limit line to 80 char
> > >>>    examples/l2fwd: space required between elements
> > >>>    examples/l2fwd: remove null initialisation
> > >> Hi Anoob,
> > >>
> > >> I am not sure if it is good idea to get syntax only fixes, I would
> > >> prefer to get syntax fixes when some other code touches that area.
> > > I'm preparing a new copy of l2fwd with support for eventmode(as
> > > suggested in,
> > > https://mails.dpdk.org/archives/dev/2018-August/109717.html). All
> > > these issues were flagged when I did the copy. What would be the
> > > better approach in that case? Fix it in just the new application or
> > > fix the source? Fixing in just the new app would mean, the two
> > > versions will have a diff at the time of copy.
> >
> > I see, make sense to not create syntax diff copied and original
> > versions, I missed the new copy part. So OK for this patch.
> 
> It was said that there will be not so much common code.
> So why bothering to reformat the original example?

There will be common code. In fact most of the code could actually be shared. But we are creating a copy and starting from there because Bruce didn't want any changes in the existing app.

> Anyway, if such cleanup is worth before duplicating, please insert it in the same
> patchset as the new example.

These fixes will be there in the new app. I'll add this in the patchset when I share the new app.

Thanks,
Anoob