[libcamera-devel] [PATCH v3 1/8] libcamera: utils: Define BIT() macro

[Jacopo Mondi]

Hi Laurent,
   I think I can live without this, I'll leave it out

Thanks
   j

On Mon, Apr 08, 2019 at 03:53:27PM +0300, Laurent Pinchart wrote:
> Hi Jacopo,
>
> Thank you for the patch.
>
> On Mon, Apr 08, 2019 at 09:46:37AM +0200, Jacopo Mondi wrote:
> > On Fri, Apr 05, 2019 at 02:31:13PM +0300, Laurent Pinchart wrote:
> > > On Wed, Apr 03, 2019 at 05:07:28PM +0200, Jacopo Mondi wrote:
> > >> Define BIT(b_) macro which expands to a left bit shift.
> > >>
> > >> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> > >> ---
> > >>  src/libcamera/include/utils.h | 1 +
> > >>  src/libcamera/utils.cpp       | 5 +++++
> > >>  2 files changed, 6 insertions(+)
> > >>
> > >> diff --git a/src/libcamera/include/utils.h b/src/libcamera/include/utils.h
> > >> index 79038a96feab..1a6cf7f7b9dc 100644
> > >> --- a/src/libcamera/include/utils.h
> > >> +++ b/src/libcamera/include/utils.h
> > >> @@ -10,6 +10,7 @@
> > >>  #include <memory>
> > >>
> > >>  #define ARRAY_SIZE(a)	(sizeof(a) / sizeof(a[0]))
> > >> +#define BIT(b_)		(1 << (b_))
> > >
> > > I'm curious, why the _ ? I'm also curious, why do you think we need this
> >
> > To avoid (unlikely) name clashes with other 'b' variables?
>
> I don't think that's needed, the compiler will not mistake the macro
> argument name with local variables named 'b'.
>
> > > macro ? :-) Usage of BIT() in the Linux kernel is partly to fix problems
> > > with (1 << 31) on 32-bit platforms, with BIT() defined as (1UL << (nr)).
> >
> > I just wanted something to avoid typing (1 << x) all the times, and
> > we're all used to the BIT() macro, so...
> >
> > I could drop this if it feels not necessary...
>
> I'm OK keeping it if you think that's useful, but I think you should
> then at least replace 1 with 1UL for the reason above.
>
> > >>  namespace libcamera {
> > >>
> > >> diff --git a/src/libcamera/utils.cpp b/src/libcamera/utils.cpp
> > >> index cd0fd7614cc7..1a5a2a03b1ca 100644
> > >> --- a/src/libcamera/utils.cpp
> > >> +++ b/src/libcamera/utils.cpp
> > >> @@ -24,6 +24,11 @@ namespace utils {
> > >>   * \brief Determine the number of elements in the static array.
> > >>   */
> > >>
> > >> +/**
> > >> + * \def BIT(b)
> > >> + * \brief Bitwise left shift by \a b bits
> > >
> > > Bitwise left shift of what ? :-)
> > >
> > >> + */
> > >> +
> > >>  /**
> > >>   * \brief Strip the directory prefix from the path
> > >>   * \param[in] path The path to process
>
> --
> Regards,
>
> Laurent Pinchart
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20190408/06525998/attachment.sig>