That seems like a rather artificial means of working around the tool rather
than having the tool reflect good programming practice.  There may be other
good reasons for inverting the comparison like that (catching = vs ==
errors, though the compiler will issue warnings for those anyway) but
circumventing checkpatch doesn't seem to be a reason we should encourage.

I personally have no issue with "redundant" code that improves clarity,
such as adding an extra set of parentheses on complex conditionals even
when they're strictly not needed (if you're a C lawyer), or making explicit
(in)equality tests with NULL or 0.

On Fri, Sep 11, 2015 at 9:13 AM, Stuart Haslam <[email protected]>
wrote:

> On Fri, Sep 11, 2015 at 01:07:47PM +0000, Savolainen, Petri (Nokia -
> FI/Espoo) wrote:
> > Hi,
> >
> > I think we need to suppress the following checkpatch CHECK level
> warning. When an API, e.g. odp_shm_addr() defines NULL as a return value,
> an application should be able to do ...
> >
> > addr = odp_shm_addr(shm);
> >
> > if (addr == NULL)
>
> You can use;
>
> if (NULL == addr)
>
> Which checkpatch doesn't complain about.
>
> >
> >
> > .. instead of ...
> >
> >
> > if (!addr)
> >
> >
> > ... which seems to be the new favorite style of checkpatch. In theory,
> NULL can be something else than 0 (in practice it's nearly always 0), but
> it's defined exactly for this use case and we should be able use it (in API
> and apps).
> >
> >
> > -Petri
> >
> >
> >
> >
> > v2-0002-api-thread-added-thread-count-max.patch has no obvious style
> problems and is ready for submission.
> > CHECK: Comparison to NULL could be written "!gbl_args->rx_stats"
> > #259: FILE: test/performance/odp_pktio_perf.c:1020:
> > +     if (gbl_args->rx_stats == NULL)
> >
> > CHECK: Comparison to NULL could be written "!gbl_args->tx_stats"
> > #270: FILE: test/performance/odp_pktio_perf.c:1031:
> > +     if (gbl_args->tx_stats == NULL)
> >
> >
> >
> > +     shm = odp_shm_reserve("test_globals.rx_stats",
> > +                           gbl_args->rx_stats_size,
> > +                           ODP_CACHE_LINE_SIZE, 0);
> > +
> > +     gbl_args->rx_stats = odp_shm_addr(shm);
> > +
> > +     if (gbl_args->rx_stats == NULL)
> > +             LOG_ABORT("Shared memory reserve failed.\n");
> >
> >
> >
> _______________________________________________
> lng-odp mailing list
> [email protected]
> https://lists.linaro.org/mailman/listinfo/lng-odp
>
_______________________________________________
lng-odp mailing list
[email protected]
https://lists.linaro.org/mailman/listinfo/lng-odp

Reply via email to