diff mbox

[1/3] linux-generic: buffer_internal add clang-format barriers

Message ID 1440001229-22483-1-git-send-email-mike.holmes@linaro.org
State New
Headers show

Commit Message

Mike Holmes Aug. 19, 2015, 4:20 p.m. UTC
The odp_format tool is not infallible, in this case it is better to omit
the macro from formatting

Signed-off-by: Mike Holmes <mike.holmes@linaro.org>
---
 platform/linux-generic/include/odp_buffer_internal.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Bill Fischofer Aug. 19, 2015, 5:26 p.m. UTC | #1
Is clang-format intended to be used as a convenience or as something that
is a standard part of ODP?  I have no problem with offering it as a
convenience to help submitters in creating clean patches, but what's the
point of having a style guide if code is going to be run through a
formatter?  That would suggest that folks really don't need to pay
attention to style since they can assume that some tools is going to
reformat things anyway.

We want code to be written to the ODP style, not converted into that for
publication from the "real" style used by each author.

On Wed, Aug 19, 2015 at 11:20 AM, Mike Holmes <mike.holmes@linaro.org>
wrote:

> The odp_format tool is not infallible, in this case it is better to omit
> the macro from formatting
>
> Signed-off-by: Mike Holmes <mike.holmes@linaro.org>
> ---
>  platform/linux-generic/include/odp_buffer_internal.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/platform/linux-generic/include/odp_buffer_internal.h
> b/platform/linux-generic/include/odp_buffer_internal.h
> index a7638da..db9defa 100644
> --- a/platform/linux-generic/include/odp_buffer_internal.h
> +++ b/platform/linux-generic/include/odp_buffer_internal.h
> @@ -30,7 +30,7 @@ extern "C" {
>  #include <odp/thread.h>
>  #include <odp/event.h>
>
> -
> +/* clang-format off */
>  #define ODP_BITSIZE(x) \
>         ((x) <=     2 ?  1 : \
>         ((x) <=     4 ?  2 : \
> @@ -49,6 +49,7 @@ extern "C" {
>         ((x) <= 32768 ? 15 : \
>         ((x) <= 65536 ? 16 : \
>          (0/0)))))))))))))))))
> +/* clang-format on */
>
>  _ODP_STATIC_ASSERT(ODP_CONFIG_PACKET_SEG_LEN_MIN >= 256,
>                    "ODP Segment size must be a minimum of 256 bytes");
> --
> 2.1.4
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/lng-odp
>
Mike Holmes Aug. 19, 2015, 6:50 p.m. UTC | #2
On 19 August 2015 at 13:26, Bill Fischofer <bill.fischofer@linaro.org> wrote:
> Is clang-format intended to be used as a convenience or as something that is
> a standard part of ODP?

I don’t see it as mandatory, just to help those who find their
favourite editor does not play well with checkpatch rules.
It also fixes 99% of the current mess generated when we moved to the
newer checkpatch automatically, I expect to run it and fix all those
warnings in 1.3 as Nicloas had proposed.

 I have no problem with offering it as a convenience
> to help submitters in creating clean patches, but what's the point of having
> a style guide if code is going to be run through a formatter?
 That would
> suggest that folks really don't need to pay attention to style since they
> can assume that some tools is going to reformat things anyway.
>
> We want code to be written to the ODP style, not converted into that for
> publication from the "real" style used by each author.

By providing this as an option those with editors that are correctly
set to a style that is generating different but still checkpatch
compliant code will be fine.
We don’t need to enforce this single tools view of the code,
checkpatch already enforces the ODP rule.

>
> On Wed, Aug 19, 2015 at 11:20 AM, Mike Holmes <mike.holmes@linaro.org>
> wrote:
>>
>> The odp_format tool is not infallible, in this case it is better to omit
>> the macro from formatting
>>
>> Signed-off-by: Mike Holmes <mike.holmes@linaro.org>
>> ---
>>  platform/linux-generic/include/odp_buffer_internal.h | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/platform/linux-generic/include/odp_buffer_internal.h
>> b/platform/linux-generic/include/odp_buffer_internal.h
>> index a7638da..db9defa 100644
>> --- a/platform/linux-generic/include/odp_buffer_internal.h
>> +++ b/platform/linux-generic/include/odp_buffer_internal.h
>> @@ -30,7 +30,7 @@ extern "C" {
>>  #include <odp/thread.h>
>>  #include <odp/event.h>
>>
>> -
>> +/* clang-format off */
>>  #define ODP_BITSIZE(x) \
>>         ((x) <=     2 ?  1 : \
>>         ((x) <=     4 ?  2 : \
>> @@ -49,6 +49,7 @@ extern "C" {
>>         ((x) <= 32768 ? 15 : \
>>         ((x) <= 65536 ? 16 : \
>>          (0/0)))))))))))))))))
>> +/* clang-format on */
>>
>>  _ODP_STATIC_ASSERT(ODP_CONFIG_PACKET_SEG_LEN_MIN >= 256,
>>                    "ODP Segment size must be a minimum of 256 bytes");
>> --
>> 2.1.4
>>
>> _______________________________________________
>> lng-odp mailing list
>> lng-odp@lists.linaro.org
>> https://lists.linaro.org/mailman/listinfo/lng-odp
>
>
Bill Fischofer Aug. 19, 2015, 7:09 p.m. UTC | #3
Ok,  then I don't see any need to "protect" existing code that's already
checkpatch clean.

On Wednesday, August 19, 2015, Mike Holmes <mike.holmes@linaro.org> wrote:

> On 19 August 2015 at 13:26, Bill Fischofer <bill.fischofer@linaro.org
> <javascript:;>> wrote:
> > Is clang-format intended to be used as a convenience or as something
> that is
> > a standard part of ODP?
>
> I don’t see it as mandatory, just to help those who find their
> favourite editor does not play well with checkpatch rules.
> It also fixes 99% of the current mess generated when we moved to the
> newer checkpatch automatically, I expect to run it and fix all those
> warnings in 1.3 as Nicloas had proposed.
>
>  I have no problem with offering it as a convenience
> > to help submitters in creating clean patches, but what's the point of
> having
> > a style guide if code is going to be run through a formatter?
>  That would
> > suggest that folks really don't need to pay attention to style since they
> > can assume that some tools is going to reformat things anyway.
> >
> > We want code to be written to the ODP style, not converted into that for
> > publication from the "real" style used by each author.
>
> By providing this as an option those with editors that are correctly
> set to a style that is generating different but still checkpatch
> compliant code will be fine.
> We don’t need to enforce this single tools view of the code,
> checkpatch already enforces the ODP rule.
>
> >
> > On Wed, Aug 19, 2015 at 11:20 AM, Mike Holmes <mike.holmes@linaro.org
> <javascript:;>>
> > wrote:
> >>
> >> The odp_format tool is not infallible, in this case it is better to omit
> >> the macro from formatting
> >>
> >> Signed-off-by: Mike Holmes <mike.holmes@linaro.org <javascript:;>>
> >> ---
> >>  platform/linux-generic/include/odp_buffer_internal.h | 3 ++-
> >>  1 file changed, 2 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/platform/linux-generic/include/odp_buffer_internal.h
> >> b/platform/linux-generic/include/odp_buffer_internal.h
> >> index a7638da..db9defa 100644
> >> --- a/platform/linux-generic/include/odp_buffer_internal.h
> >> +++ b/platform/linux-generic/include/odp_buffer_internal.h
> >> @@ -30,7 +30,7 @@ extern "C" {
> >>  #include <odp/thread.h>
> >>  #include <odp/event.h>
> >>
> >> -
> >> +/* clang-format off */
> >>  #define ODP_BITSIZE(x) \
> >>         ((x) <=     2 ?  1 : \
> >>         ((x) <=     4 ?  2 : \
> >> @@ -49,6 +49,7 @@ extern "C" {
> >>         ((x) <= 32768 ? 15 : \
> >>         ((x) <= 65536 ? 16 : \
> >>          (0/0)))))))))))))))))
> >> +/* clang-format on */
> >>
> >>  _ODP_STATIC_ASSERT(ODP_CONFIG_PACKET_SEG_LEN_MIN >= 256,
> >>                    "ODP Segment size must be a minimum of 256 bytes");
> >> --
> >> 2.1.4
> >>
> >> _______________________________________________
> >> lng-odp mailing list
> >> lng-odp@lists.linaro.org <javascript:;>
> >> https://lists.linaro.org/mailman/listinfo/lng-odp
> >
> >
>
>
>
> --
> Mike Holmes
> Technical Manager - Linaro Networking Group
> Linaro.org │ Open source software for ARM SoCs
>
Mike Holmes Aug. 19, 2015, 7:24 p.m. UTC | #4
On 19 August 2015 at 15:09, Bill Fischofer <bill.fischofer@linaro.org> wrote:
> Ok,  then I don't see any need to "protect" existing code that's already
> checkpatch clean.

It is needed when we run this to clean up the existing problems - seen
here https://ci.linaro.org/job/odp-api-style-check/label=build/534/console
It is the only case in the entire code base where there is a conflict
and the result looks worse although it is complaint and does compile,
by marking it future changes to this file can be run though and will
not cause a problem.

>
>
> On Wednesday, August 19, 2015, Mike Holmes <mike.holmes@linaro.org> wrote:
>>
>> On 19 August 2015 at 13:26, Bill Fischofer <bill.fischofer@linaro.org>
>> wrote:
>> > Is clang-format intended to be used as a convenience or as something
>> > that is
>> > a standard part of ODP?
>>
>> I don’t see it as mandatory, just to help those who find their
>> favourite editor does not play well with checkpatch rules.
>> It also fixes 99% of the current mess generated when we moved to the
>> newer checkpatch automatically, I expect to run it and fix all those
>> warnings in 1.3 as Nicloas had proposed.
>>
>>  I have no problem with offering it as a convenience
>> > to help submitters in creating clean patches, but what's the point of
>> > having
>> > a style guide if code is going to be run through a formatter?
>>  That would
>> > suggest that folks really don't need to pay attention to style since
>> > they
>> > can assume that some tools is going to reformat things anyway.
>> >
>> > We want code to be written to the ODP style, not converted into that for
>> > publication from the "real" style used by each author.
>>
>> By providing this as an option those with editors that are correctly
>> set to a style that is generating different but still checkpatch
>> compliant code will be fine.
>> We don’t need to enforce this single tools view of the code,
>> checkpatch already enforces the ODP rule.
>>
>> >
>> > On Wed, Aug 19, 2015 at 11:20 AM, Mike Holmes <mike.holmes@linaro.org>
>> > wrote:
>> >>
>> >> The odp_format tool is not infallible, in this case it is better to
>> >> omit
>> >> the macro from formatting
>> >>
>> >> Signed-off-by: Mike Holmes <mike.holmes@linaro.org>
>> >> ---
>> >>  platform/linux-generic/include/odp_buffer_internal.h | 3 ++-
>> >>  1 file changed, 2 insertions(+), 1 deletion(-)
>> >>
>> >> diff --git a/platform/linux-generic/include/odp_buffer_internal.h
>> >> b/platform/linux-generic/include/odp_buffer_internal.h
>> >> index a7638da..db9defa 100644
>> >> --- a/platform/linux-generic/include/odp_buffer_internal.h
>> >> +++ b/platform/linux-generic/include/odp_buffer_internal.h
>> >> @@ -30,7 +30,7 @@ extern "C" {
>> >>  #include <odp/thread.h>
>> >>  #include <odp/event.h>
>> >>
>> >> -
>> >> +/* clang-format off */
>> >>  #define ODP_BITSIZE(x) \
>> >>         ((x) <=     2 ?  1 : \
>> >>         ((x) <=     4 ?  2 : \
>> >> @@ -49,6 +49,7 @@ extern "C" {
>> >>         ((x) <= 32768 ? 15 : \
>> >>         ((x) <= 65536 ? 16 : \
>> >>          (0/0)))))))))))))))))
>> >> +/* clang-format on */
>> >>
>> >>  _ODP_STATIC_ASSERT(ODP_CONFIG_PACKET_SEG_LEN_MIN >= 256,
>> >>                    "ODP Segment size must be a minimum of 256 bytes");
>> >> --
>> >> 2.1.4
>> >>
>> >> _______________________________________________
>> >> lng-odp mailing list
>> >> lng-odp@lists.linaro.org
>> >> https://lists.linaro.org/mailman/listinfo/lng-odp
>> >
>> >
>>
>>
>>
>> --
>> Mike Holmes
>> Technical Manager - Linaro Networking Group
>> Linaro.org │ Open source software for ARM SoCs
Bill Fischofer Aug. 19, 2015, 7:29 p.m. UTC | #5
It's the responsibility of each patch submitter to ensure that the
submission is checkpatch clean.  If we're doing a one-off scrub of the
existing code then any anomalies can be dealt with via one-off manual edits
so that the cleanup patches are clean. From then on normal patch processes
should maintain conformance.

On Wednesday, August 19, 2015, Mike Holmes <mike.holmes@linaro.org> wrote:

> On 19 August 2015 at 15:09, Bill Fischofer <bill.fischofer@linaro.org
> <javascript:;>> wrote:
> > Ok,  then I don't see any need to "protect" existing code that's already
> > checkpatch clean.
>
> It is needed when we run this to clean up the existing problems - seen
> here https://ci.linaro.org/job/odp-api-style-check/label=build/534/console
> It is the only case in the entire code base where there is a conflict
> and the result looks worse although it is complaint and does compile,
> by marking it future changes to this file can be run though and will
> not cause a problem.
>
> >
> >
> > On Wednesday, August 19, 2015, Mike Holmes <mike.holmes@linaro.org
> <javascript:;>> wrote:
> >>
> >> On 19 August 2015 at 13:26, Bill Fischofer <bill.fischofer@linaro.org
> <javascript:;>>
> >> wrote:
> >> > Is clang-format intended to be used as a convenience or as something
> >> > that is
> >> > a standard part of ODP?
> >>
> >> I don’t see it as mandatory, just to help those who find their
> >> favourite editor does not play well with checkpatch rules.
> >> It also fixes 99% of the current mess generated when we moved to the
> >> newer checkpatch automatically, I expect to run it and fix all those
> >> warnings in 1.3 as Nicloas had proposed.
> >>
> >>  I have no problem with offering it as a convenience
> >> > to help submitters in creating clean patches, but what's the point of
> >> > having
> >> > a style guide if code is going to be run through a formatter?
> >>  That would
> >> > suggest that folks really don't need to pay attention to style since
> >> > they
> >> > can assume that some tools is going to reformat things anyway.
> >> >
> >> > We want code to be written to the ODP style, not converted into that
> for
> >> > publication from the "real" style used by each author.
> >>
> >> By providing this as an option those with editors that are correctly
> >> set to a style that is generating different but still checkpatch
> >> compliant code will be fine.
> >> We don’t need to enforce this single tools view of the code,
> >> checkpatch already enforces the ODP rule.
> >>
> >> >
> >> > On Wed, Aug 19, 2015 at 11:20 AM, Mike Holmes <mike.holmes@linaro.org
> <javascript:;>>
> >> > wrote:
> >> >>
> >> >> The odp_format tool is not infallible, in this case it is better to
> >> >> omit
> >> >> the macro from formatting
> >> >>
> >> >> Signed-off-by: Mike Holmes <mike.holmes@linaro.org <javascript:;>>
> >> >> ---
> >> >>  platform/linux-generic/include/odp_buffer_internal.h | 3 ++-
> >> >>  1 file changed, 2 insertions(+), 1 deletion(-)
> >> >>
> >> >> diff --git a/platform/linux-generic/include/odp_buffer_internal.h
> >> >> b/platform/linux-generic/include/odp_buffer_internal.h
> >> >> index a7638da..db9defa 100644
> >> >> --- a/platform/linux-generic/include/odp_buffer_internal.h
> >> >> +++ b/platform/linux-generic/include/odp_buffer_internal.h
> >> >> @@ -30,7 +30,7 @@ extern "C" {
> >> >>  #include <odp/thread.h>
> >> >>  #include <odp/event.h>
> >> >>
> >> >> -
> >> >> +/* clang-format off */
> >> >>  #define ODP_BITSIZE(x) \
> >> >>         ((x) <=     2 ?  1 : \
> >> >>         ((x) <=     4 ?  2 : \
> >> >> @@ -49,6 +49,7 @@ extern "C" {
> >> >>         ((x) <= 32768 ? 15 : \
> >> >>         ((x) <= 65536 ? 16 : \
> >> >>          (0/0)))))))))))))))))
> >> >> +/* clang-format on */
> >> >>
> >> >>  _ODP_STATIC_ASSERT(ODP_CONFIG_PACKET_SEG_LEN_MIN >= 256,
> >> >>                    "ODP Segment size must be a minimum of 256
> bytes");
> >> >> --
> >> >> 2.1.4
> >> >>
> >> >> _______________________________________________
> >> >> lng-odp mailing list
> >> >> lng-odp@lists.linaro.org <javascript:;>
> >> >> https://lists.linaro.org/mailman/listinfo/lng-odp
> >> >
> >> >
> >>
> >>
> >>
> >> --
> >> Mike Holmes
> >> Technical Manager - Linaro Networking Group
> >> Linaro.org │ Open source software for ARM SoCs
>
>
>
> --
> Mike Holmes
> Technical Manager - Linaro Networking Group
> Linaro.org │ Open source software for ARM SoCs
>
Maxim Uvarov Aug. 24, 2015, 11:13 a.m. UTC | #6
On 08/19/15 19:20, Mike Holmes wrote:
> The odp_format tool is not infallible, in this case it is better to omit
> the macro from formatting
>
> Signed-off-by: Mike Holmes <mike.holmes@linaro.org>
> ---
>   platform/linux-generic/include/odp_buffer_internal.h | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/platform/linux-generic/include/odp_buffer_internal.h b/platform/linux-generic/include/odp_buffer_internal.h
> index a7638da..db9defa 100644
> --- a/platform/linux-generic/include/odp_buffer_internal.h
> +++ b/platform/linux-generic/include/odp_buffer_internal.h
> @@ -30,7 +30,7 @@ extern "C" {
>   #include <odp/thread.h>
>   #include <odp/event.h>
>   
> -
> +/* clang-format off */
>   #define ODP_BITSIZE(x) \
>   	((x) <=     2 ?  1 : \
>   	((x) <=     4 ?  2 : \
> @@ -49,6 +49,7 @@ extern "C" {
>   	((x) <= 32768 ? 15 : \
>   	((x) <= 65536 ? 16 : \
>   	 (0/0)))))))))))))))))
> +/* clang-format on */
>   
that is ugly solution. We should not add to code comments specific to 
different tools. Can that be in some exception file?

Maxim.

>   _ODP_STATIC_ASSERT(ODP_CONFIG_PACKET_SEG_LEN_MIN >= 256,
>   		   "ODP Segment size must be a minimum of 256 bytes");
Maxim Uvarov Aug. 24, 2015, 11:24 a.m. UTC | #7
It's not clear who is the user of that tool. Sources needs to be clean 
only once and after that we can forgot about it.
And it does not matter which tool you used to format any sources. Result 
is important. If other platforms need to
clean sources and you want to maintain that script,  you can commit it 
to odp-check.git.

Maxim.

On 08/19/15 22:29, Bill Fischofer wrote:
> It's the responsibility of each patch submitter to ensure that the 
> submission is checkpatch clean.  If we're doing a one-off scrub of the 
> existing code then any anomalies can be dealt with via one-off manual 
> edits so that the cleanup patches are clean. From then on normal patch 
> processes should maintain conformance.
>
> On Wednesday, August 19, 2015, Mike Holmes <mike.holmes@linaro.org 
> <mailto:mike.holmes@linaro.org>> wrote:
>
>     On 19 August 2015 at 15:09, Bill Fischofer
>     <bill.fischofer@linaro.org <javascript:;>> wrote:
>     > Ok,  then I don't see any need to "protect" existing code that's
>     already
>     > checkpatch clean.
>
>     It is needed when we run this to clean up the existing problems - seen
>     here
>     https://ci.linaro.org/job/odp-api-style-check/label=build/534/console
>     It is the only case in the entire code base where there is a conflict
>     and the result looks worse although it is complaint and does compile,
>     by marking it future changes to this file can be run though and will
>     not cause a problem.
>
>     >
>     >
>     > On Wednesday, August 19, 2015, Mike Holmes
>     <mike.holmes@linaro.org <javascript:;>> wrote:
>     >>
>     >> On 19 August 2015 at 13:26, Bill Fischofer
>     <bill.fischofer@linaro.org <javascript:;>>
>     >> wrote:
>     >> > Is clang-format intended to be used as a convenience or as
>     something
>     >> > that is
>     >> > a standard part of ODP?
>     >>
>     >> I don’t see it as mandatory, just to help those who find their
>     >> favourite editor does not play well with checkpatch rules.
>     >> It also fixes 99% of the current mess generated when we moved
>     to the
>     >> newer checkpatch automatically, I expect to run it and fix all
>     those
>     >> warnings in 1.3 as Nicloas had proposed.
>     >>
>     >>  I have no problem with offering it as a convenience
>     >> > to help submitters in creating clean patches, but what's the
>     point of
>     >> > having
>     >> > a style guide if code is going to be run through a formatter?
>     >>  That would
>     >> > suggest that folks really don't need to pay attention to
>     style since
>     >> > they
>     >> > can assume that some tools is going to reformat things anyway.
>     >> >
>     >> > We want code to be written to the ODP style, not converted
>     into that for
>     >> > publication from the "real" style used by each author.
>     >>
>     >> By providing this as an option those with editors that are
>     correctly
>     >> set to a style that is generating different but still checkpatch
>     >> compliant code will be fine.
>     >> We don’t need to enforce this single tools view of the code,
>     >> checkpatch already enforces the ODP rule.
>     >>
>     >> >
>     >> > On Wed, Aug 19, 2015 at 11:20 AM, Mike Holmes
>     <mike.holmes@linaro.org <javascript:;>>
>     >> > wrote:
>     >> >>
>     >> >> The odp_format tool is not infallible, in this case it is
>     better to
>     >> >> omit
>     >> >> the macro from formatting
>     >> >>
>     >> >> Signed-off-by: Mike Holmes <mike.holmes@linaro.org
>     <javascript:;>>
>     >> >> ---
>     >> >> platform/linux-generic/include/odp_buffer_internal.h | 3 ++-
>     >> >>  1 file changed, 2 insertions(+), 1 deletion(-)
>     >> >>
>     >> >> diff --git
>     a/platform/linux-generic/include/odp_buffer_internal.h
>     >> >> b/platform/linux-generic/include/odp_buffer_internal.h
>     >> >> index a7638da..db9defa 100644
>     >> >> --- a/platform/linux-generic/include/odp_buffer_internal.h
>     >> >> +++ b/platform/linux-generic/include/odp_buffer_internal.h
>     >> >> @@ -30,7 +30,7 @@ extern "C" {
>     >> >>  #include <odp/thread.h>
>     >> >>  #include <odp/event.h>
>     >> >>
>     >> >> -
>     >> >> +/* clang-format off */
>     >> >>  #define ODP_BITSIZE(x) \
>     >> >>         ((x) <=     2 ?  1 : \
>     >> >>         ((x) <=     4 ?  2 : \
>     >> >> @@ -49,6 +49,7 @@ extern "C" {
>     >> >>         ((x) <= 32768 ? 15 : \
>     >> >>         ((x) <= 65536 ? 16 : \
>     >> >>          (0/0)))))))))))))))))
>     >> >> +/* clang-format on */
>     >> >>
>     >> >> _ODP_STATIC_ASSERT(ODP_CONFIG_PACKET_SEG_LEN_MIN >= 256,
>     >> >>                    "ODP Segment size must be a minimum of
>     256 bytes");
>     >> >> --
>     >> >> 2.1.4
>     >> >>
>     >> >> _______________________________________________
>     >> >> lng-odp mailing list
>     >> >> lng-odp@lists.linaro.org <javascript:;>
>     >> >> https://lists.linaro.org/mailman/listinfo/lng-odp
>     >> >
>     >> >
>     >>
>     >>
>     >>
>     >> --
>     >> Mike Holmes
>     >> Technical Manager - Linaro Networking Group
>     >> Linaro.org │ Open source software for ARM SoCs
>
>
>
>     --
>     Mike Holmes
>     Technical Manager - Linaro Networking Group
>     Linaro.org │ Open source software for ARM SoCs
>
>
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/lng-odp
Mike Holmes Aug. 24, 2015, 8:38 p.m. UTC | #8
On 24 August 2015 at 07:24, Maxim Uvarov <maxim.uvarov@linaro.org> wrote:
> It's not clear who is the user of that tool. Sources needs to be clean only
> once and after that we can forgot about it.
> And it does not matter which tool you used to format any sources. Result is
> important. If other platforms need to
> clean sources and you want to maintain that script,  you can commit it to
> odp-check.git.

Agreed any tool can be used, that is why this is not "the tool"  just a tool.

I think this is much better in ODP, it is the partner to checkpatch
that caused all the problems to start with and it will happen again
with the next update to checkpatch I expect.
But we don’t have to put in barriers, I don’t think that is crucial.

>
> Maxim.
>
> On 08/19/15 22:29, Bill Fischofer wrote:
>>
>> It's the responsibility of each patch submitter to ensure that the
>> submission is checkpatch clean.  If we're doing a one-off scrub of the
>> existing code then any anomalies can be dealt with via one-off manual edits
>> so that the cleanup patches are clean. From then on normal patch processes
>> should maintain conformance.
>>
>> On Wednesday, August 19, 2015, Mike Holmes <mike.holmes@linaro.org
>> <mailto:mike.holmes@linaro.org>> wrote:
>>
>>     On 19 August 2015 at 15:09, Bill Fischofer
>>     <bill.fischofer@linaro.org <javascript:;>> wrote:
>>     > Ok,  then I don't see any need to "protect" existing code that's
>>     already
>>     > checkpatch clean.
>>
>>     It is needed when we run this to clean up the existing problems - seen
>>     here
>>     https://ci.linaro.org/job/odp-api-style-check/label=build/534/console
>>     It is the only case in the entire code base where there is a conflict
>>     and the result looks worse although it is complaint and does compile,
>>     by marking it future changes to this file can be run though and will
>>     not cause a problem.
>>
>>     >
>>     >
>>     > On Wednesday, August 19, 2015, Mike Holmes
>>     <mike.holmes@linaro.org <javascript:;>> wrote:
>>     >>
>>     >> On 19 August 2015 at 13:26, Bill Fischofer
>>     <bill.fischofer@linaro.org <javascript:;>>
>>
>>     >> wrote:
>>     >> > Is clang-format intended to be used as a convenience or as
>>     something
>>     >> > that is
>>     >> > a standard part of ODP?
>>     >>
>>     >> I don’t see it as mandatory, just to help those who find their
>>     >> favourite editor does not play well with checkpatch rules.
>>     >> It also fixes 99% of the current mess generated when we moved
>>     to the
>>     >> newer checkpatch automatically, I expect to run it and fix all
>>     those
>>     >> warnings in 1.3 as Nicloas had proposed.
>>     >>
>>     >>  I have no problem with offering it as a convenience
>>     >> > to help submitters in creating clean patches, but what's the
>>     point of
>>     >> > having
>>     >> > a style guide if code is going to be run through a formatter?
>>     >>  That would
>>     >> > suggest that folks really don't need to pay attention to
>>     style since
>>     >> > they
>>     >> > can assume that some tools is going to reformat things anyway.
>>     >> >
>>     >> > We want code to be written to the ODP style, not converted
>>     into that for
>>     >> > publication from the "real" style used by each author.
>>     >>
>>     >> By providing this as an option those with editors that are
>>     correctly
>>     >> set to a style that is generating different but still checkpatch
>>     >> compliant code will be fine.
>>     >> We don’t need to enforce this single tools view of the code,
>>     >> checkpatch already enforces the ODP rule.
>>     >>
>>     >> >
>>     >> > On Wed, Aug 19, 2015 at 11:20 AM, Mike Holmes
>>     <mike.holmes@linaro.org <javascript:;>>
>>     >> > wrote:
>>     >> >>
>>     >> >> The odp_format tool is not infallible, in this case it is
>>     better to
>>     >> >> omit
>>     >> >> the macro from formatting
>>     >> >>
>>     >> >> Signed-off-by: Mike Holmes <mike.holmes@linaro.org
>>     <javascript:;>>
>>
>>     >> >> ---
>>     >> >> platform/linux-generic/include/odp_buffer_internal.h | 3 ++-
>>     >> >>  1 file changed, 2 insertions(+), 1 deletion(-)
>>     >> >>
>>     >> >> diff --git
>>     a/platform/linux-generic/include/odp_buffer_internal.h
>>     >> >> b/platform/linux-generic/include/odp_buffer_internal.h
>>     >> >> index a7638da..db9defa 100644
>>     >> >> --- a/platform/linux-generic/include/odp_buffer_internal.h
>>     >> >> +++ b/platform/linux-generic/include/odp_buffer_internal.h
>>     >> >> @@ -30,7 +30,7 @@ extern "C" {
>>     >> >>  #include <odp/thread.h>
>>     >> >>  #include <odp/event.h>
>>     >> >>
>>     >> >> -
>>     >> >> +/* clang-format off */
>>     >> >>  #define ODP_BITSIZE(x) \
>>     >> >>         ((x) <=     2 ?  1 : \
>>     >> >>         ((x) <=     4 ?  2 : \
>>     >> >> @@ -49,6 +49,7 @@ extern "C" {
>>     >> >>         ((x) <= 32768 ? 15 : \
>>     >> >>         ((x) <= 65536 ? 16 : \
>>     >> >>          (0/0)))))))))))))))))
>>     >> >> +/* clang-format on */
>>     >> >>
>>     >> >> _ODP_STATIC_ASSERT(ODP_CONFIG_PACKET_SEG_LEN_MIN >= 256,
>>     >> >>                    "ODP Segment size must be a minimum of
>>     256 bytes");
>>     >> >> --
>>     >> >> 2.1.4
>>     >> >>
>>     >> >> _______________________________________________
>>     >> >> lng-odp mailing list
>>     >> >> lng-odp@lists.linaro.org <javascript:;>
>>     >> >> https://lists.linaro.org/mailman/listinfo/lng-odp
>>     >> >
>>     >> >
>>     >>
>>     >>
>>     >>
>>     >> --
>>     >> Mike Holmes
>>     >> Technical Manager - Linaro Networking Group
>>     >> Linaro.org │ Open source software for ARM SoCs
>>
>>
>>
>>     --
>>     Mike Holmes
>>     Technical Manager - Linaro Networking Group
>>     Linaro.org │ Open source software for ARM SoCs
>>
>>
>>
>> _______________________________________________
>> lng-odp mailing list
>> lng-odp@lists.linaro.org
>> https://lists.linaro.org/mailman/listinfo/lng-odp
>
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/lng-odp
diff mbox

Patch

diff --git a/platform/linux-generic/include/odp_buffer_internal.h b/platform/linux-generic/include/odp_buffer_internal.h
index a7638da..db9defa 100644
--- a/platform/linux-generic/include/odp_buffer_internal.h
+++ b/platform/linux-generic/include/odp_buffer_internal.h
@@ -30,7 +30,7 @@  extern "C" {
 #include <odp/thread.h>
 #include <odp/event.h>
 
-
+/* clang-format off */
 #define ODP_BITSIZE(x) \
 	((x) <=     2 ?  1 : \
 	((x) <=     4 ?  2 : \
@@ -49,6 +49,7 @@  extern "C" {
 	((x) <= 32768 ? 15 : \
 	((x) <= 65536 ? 16 : \
 	 (0/0)))))))))))))))))
+/* clang-format on */
 
 _ODP_STATIC_ASSERT(ODP_CONFIG_PACKET_SEG_LEN_MIN >= 256,
 		   "ODP Segment size must be a minimum of 256 bytes");