[libcamera-devel,09/10] android: metadata: Prevent variable aliasing

10055 diff mbox series

The validate_camera_metadata_structure() call uses two instances of a
variable named aligned_ptr().

Reuse the first instance which is not otherwise re-used for the second
scoped usage.

Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
---
*** This is in android metadata library code ***

This is only here as a temporary repair. Ideally we shouldn't make changes to this code base.
disabling the warning on this library component is likely a better choice.

 src/android/metadata/camera_metadata.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Hi Kieran,

On 2020-10-13 16:12:40 +0100, Kieran Bingham wrote:
> The validate_camera_metadata_structure() call uses two instances of a
> variable named aligned_ptr().
> 
> Reuse the first instance which is not otherwise re-used for the second
> scoped usage.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> ---
> *** This is in android metadata library code ***
> 
> This is only here as a temporary repair. Ideally we shouldn't make changes to this code base.
> disabling the warning on this library component is likely a better choice.
> 
>  src/android/metadata/camera_metadata.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/android/metadata/camera_metadata.c b/src/android/metadata/camera_metadata.c
> index b86586a7e685..129d1e92ece1 100644
> --- a/src/android/metadata/camera_metadata.c
> +++ b/src/android/metadata/camera_metadata.c
> @@ -433,7 +433,7 @@ int validate_camera_metadata_structure(const camera_metadata_t *metadata,
>          };
>  
>          for (size_t i = 0; i < sizeof(alignments)/sizeof(alignments[0]); ++i) {
> -            uintptr_t aligned_ptr = ALIGN_TO((uintptr_t) metadata + alignmentOffset,
> +            aligned_ptr = ALIGN_TO((uintptr_t) metadata + alignmentOffset,

This is a tricky one. As you say ideally we should fix this upstream and 
backport it. But being able to enable the build time warning of our own 
code base would be really nice. As we already inject a SPDX header to 
this file I see no real harm, with a patch submitted for upstream,

Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>

>                      alignments[i].alignment);
>  
>              if ((uintptr_t)metadata + alignmentOffset != aligned_ptr) {
> -- 
> 2.25.1
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

On Wed, Oct 14, 2020 at 02:42:07PM +0200, Niklas Söderlund wrote:
> On 2020-10-13 16:12:40 +0100, Kieran Bingham wrote:
> > The validate_camera_metadata_structure() call uses two instances of a
> > variable named aligned_ptr().
> > 
> > Reuse the first instance which is not otherwise re-used for the second
> > scoped usage.
> > 
> > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> > ---
> > *** This is in android metadata library code ***
> > 
> > This is only here as a temporary repair. Ideally we shouldn't make changes to this code base.
> > disabling the warning on this library component is likely a better choice.
> > 
> >  src/android/metadata/camera_metadata.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/src/android/metadata/camera_metadata.c b/src/android/metadata/camera_metadata.c
> > index b86586a7e685..129d1e92ece1 100644
> > --- a/src/android/metadata/camera_metadata.c
> > +++ b/src/android/metadata/camera_metadata.c
> > @@ -433,7 +433,7 @@ int validate_camera_metadata_structure(const camera_metadata_t *metadata,
> >          };
> >  
> >          for (size_t i = 0; i < sizeof(alignments)/sizeof(alignments[0]); ++i) {
> > -            uintptr_t aligned_ptr = ALIGN_TO((uintptr_t) metadata + alignmentOffset,
> > +            aligned_ptr = ALIGN_TO((uintptr_t) metadata + alignmentOffset,
> 
> This is a tricky one. As you say ideally we should fix this upstream and 
> backport it. But being able to enable the build time warning of our own 
> code base would be really nice. As we already inject a SPDX header to 
> this file I see no real harm, with a patch submitted for upstream,
> 
> Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>

Alternatively we could disable the warning for the Android camera
metadata library.

> >                      alignments[i].alignment);
> >  
> >              if ((uintptr_t)metadata + alignmentOffset != aligned_ptr) {