[libcamera-devel,08/10] libcamera: src: ipa: raspberrypi: agc: Fix uninitialised members in status_
diff mbox series

Message ID 20201116164918.2055-9-david.plowman@raspberrypi.com
State Superseded
Headers show
Series
  • Raspberry Pi AGC
Related show

Commit Message

David Plowman Nov. 16, 2020, 4:49 p.m. UTC
Use memset in the constructor, it is tidier and initialises
everything.

Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
---
 src/ipa/raspberrypi/controller/rpi/agc.cpp | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

Comments

Naushir Patuck Nov. 17, 2020, 10:58 a.m. UTC | #1
Hi David,

On Mon, 16 Nov 2020 at 16:49, David Plowman <david.plowman@raspberrypi.com>
wrote:

> Use memset in the constructor, it is tidier and initialises
> everything.
>
> Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
> ---
>  src/ipa/raspberrypi/controller/rpi/agc.cpp | 13 ++++++-------
>  1 file changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/src/ipa/raspberrypi/controller/rpi/agc.cpp
> b/src/ipa/raspberrypi/controller/rpi/agc.cpp
> index 7c7944e8..1037f968 100644
> --- a/src/ipa/raspberrypi/controller/rpi/agc.cpp
> +++ b/src/ipa/raspberrypi/controller/rpi/agc.cpp
> @@ -157,14 +157,13 @@ Agc::Agc(Controller *controller)
>           exposure_mode_(nullptr), constraint_mode_(nullptr),
>           frame_count_(0), lock_count_(0)
>  {
> +       // Setting status_.total_exposure_value_ to zero initially tells us
> +       // it's not been calculated yet (i.e. Process hasn't yet run).
> +       memset(&status_, 0, sizeof(status_));
>         ev_ = status_.ev = 1.0;
> -       flicker_period_ = status_.flicker_period = 0.0;
> -       fixed_shutter_ = status_.fixed_shutter = 0;
> -       fixed_analogue_gain_ = status_.fixed_analogue_gain = 0.0;
> -       // set to zero initially, so we can tell it's not been calculated
> -       status_.total_exposure_value = 0.0;
> -       status_.target_exposure_value = 0.0;
> -       status_.locked = false;
> +       flicker_period_ = 0.0;
> +       fixed_shutter_ = 0;
> +       fixed_analogue_gain_ = 0.0;
>

Minor nit, but could these be initialised by the constructor's initialiser
list?  Apart from that:

Reviewed-by: Naushir Patuck <naush@raspberrypi.com>


>  }
>
>  char const *Agc::Name() const
> --
> 2.20.1
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
>

Patch
diff mbox series

diff --git a/src/ipa/raspberrypi/controller/rpi/agc.cpp b/src/ipa/raspberrypi/controller/rpi/agc.cpp
index 7c7944e8..1037f968 100644
--- a/src/ipa/raspberrypi/controller/rpi/agc.cpp
+++ b/src/ipa/raspberrypi/controller/rpi/agc.cpp
@@ -157,14 +157,13 @@  Agc::Agc(Controller *controller)
 	  exposure_mode_(nullptr), constraint_mode_(nullptr),
 	  frame_count_(0), lock_count_(0)
 {
+	// Setting status_.total_exposure_value_ to zero initially tells us
+	// it's not been calculated yet (i.e. Process hasn't yet run).
+	memset(&status_, 0, sizeof(status_));
 	ev_ = status_.ev = 1.0;
-	flicker_period_ = status_.flicker_period = 0.0;
-	fixed_shutter_ = status_.fixed_shutter = 0;
-	fixed_analogue_gain_ = status_.fixed_analogue_gain = 0.0;
-	// set to zero initially, so we can tell it's not been calculated
-	status_.total_exposure_value = 0.0;
-	status_.target_exposure_value = 0.0;
-	status_.locked = false;
+	flicker_period_ = 0.0;
+	fixed_shutter_ = 0;
+	fixed_analogue_gain_ = 0.0;
 }
 
 char const *Agc::Name() const