I tried a modern version of checkpatch and it complained on everything! We need to spend some effort configuring checkpatch according to our needs. Our needs are necessarily not identical to the Linux kernel.
-- Ola On 10 November 2014 13:55, Mike Holmes <[email protected]> wrote: > As Maxim says we adopted the Kernel style not the C style but it is > configurable to be ODP style. > > It is easy to disable a particular warning by modifying > odp/.checkpatch.conf, we have also not updated checkpatch in a year and > that might be worth investigation. > > On 10 November 2014 07:26, Bill Fischofer <[email protected]> wrote: >> >> The main use case for anonymous bit fields is when mapping HW registers >> that have HW-reserved bits that should not be touched by SW. Another is for >> architected packet formats with explicitly reserved fields that again SW >> should not try to repurpose. Being anonymous reduces the chance of stray >> references to them and also makes it clear to all that they are not to be >> used. >> >> >> >> On Mon, Nov 10, 2014 at 6:04 AM, Maxim Uvarov <[email protected]> >> wrote: >>> >>> On 11/10/2014 03:19 AM, Ola Liljedahl wrote: >>>> >>>> Checkpatch complains on a lot of stuff that is legitimate C... And >>>> many things are considered errors when I personally think a warning >>>> should suffice. >>>> >>>> I also had a case where checkpatch complained (error I think) on the >>>> definition of a macro (something related to inline assembly), it >>>> expanded into multiple separate "terms" (e.g. __asm and __volatile) >>>> and recommended I put parenthesis around the definition. Except that >>>> that caused a syntax error when compiling. So checkpatch's knowledge >>>> of C and the GNU extensions is limited. >>>> >>>> -- Ola >>>> >>>> >>>> On 10 November 2014 00:39, Bill Fischofer <[email protected]> >>>> wrote: >>>>> >>>>> I'm working through my patch trying to get it checkpatch clean and one >>>>> of >>>>> the thingns that checkpatch is complaining about is the use of >>>>> anonymous bit >>>>> fields. For example: >>>>> >>>>> struct foo { >>>>> uint32_t :2; /* We don't care about these bits >>>>> */ >>>>> uing32_t goodstuff:12; /* The stuff we care about */ >>>>> .... >>>>> }. >>>>> >>>>> this is legitimate C but checkpatch flags this as an error. So we're >>>>> supposed to make up dummy names for what would otherwise be anonymous >>>>> bitfields? Not sure I understand the logic behind that. >>>>> >>>>> Any insights? >>>>> >>>>> Bill >>> >>> >>> Check patch was taken from kernel. Might be they had bad practice with >>> anonymous bit fields. >>> I also think that it might be confusing what are these fields and why >>> they don't used. Might be somebody >>> later will need them for some reason. So I think "int bad_unused" is >>> better than anonymous. >>> >>> Thanks, >>> Maxim. >>> >>> >>>>> _______________________________________________ >>>>> lng-odp mailing list >>>>> [email protected] >>>>> http://lists.linaro.org/mailman/listinfo/lng-odp >>>>> >>>> _______________________________________________ >>>> lng-odp mailing list >>>> [email protected] >>>> http://lists.linaro.org/mailman/listinfo/lng-odp >>> >>> >>> >>> _______________________________________________ >>> lng-odp mailing list >>> [email protected] >>> http://lists.linaro.org/mailman/listinfo/lng-odp >> >> >> >> _______________________________________________ >> lng-odp mailing list >> [email protected] >> http://lists.linaro.org/mailman/listinfo/lng-odp >> > > > > -- > Mike Holmes > Linaro Sr Technical Manager > LNG - ODP > > _______________________________________________ > lng-odp mailing list > [email protected] > http://lists.linaro.org/mailman/listinfo/lng-odp > _______________________________________________ lng-odp mailing list [email protected] http://lists.linaro.org/mailman/listinfo/lng-odp
