| Message ID | 20190403150735.27580-2-jacopo@jmondi.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Jacopo, Thanks for your patch. On 2019-04-03 17:07:28 +0200, Jacopo Mondi wrote: > Define BIT(b_) macro which expands to a left bit shift. > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > --- > 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_)) > > 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 > + */ > + > /** > * \brief Strip the directory prefix from the path > * \param[in] path The path to process > -- > 2.21.0 > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
Hi Jacopo, Thank you for the patch. 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@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 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)). > > 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
Hi Laurent, On Fri, Apr 05, 2019 at 02:31:13PM +0300, Laurent Pinchart wrote: > Hi Jacopo, > > Thank you for the patch. > > 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@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? > 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... Thanks j > > > > > 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
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@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
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@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
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_)) 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 + */ + /** * \brief Strip the directory prefix from the path * \param[in] path The path to process
Define BIT(b_) macro which expands to a left bit shift. Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> --- src/libcamera/include/utils.h | 1 + src/libcamera/utils.cpp | 5 +++++ 2 files changed, 6 insertions(+)