[U-Boot] [PATCH 2/2] libavb: Fix build warnings after updating the lib

[Eugeniu Rosca]

Hi Sam,

CC: LIBAVB people (w.r.t. libavb fixes in U-Boot)

I can reproduce the compiler warnings myself and I confirm they are
fixed with this patch. More comments below.

On Thu, Aug 15, 2019 at 11:04:03PM +0300, Sam Protsenko wrote:
> After updating libavb to most recent version from AOSP/master, two new
> warnings appear:
> 
> Warning #1:
> 
>     lib/libavb/avb_cmdline.c: In function 'avb_append_options':
>     lib/libavb/avb_cmdline.c:365:15: warning: 'dm_verity_mode' may be
>                                      used uninitialized in this function
>                                      [-Wmaybe-uninitialized]
>          new_ret = avb_replace(
>                    ^~~~~~~~~~~~
>              slot_data->cmdline, "$(ANDROID_VERITY_MODE)", dm_verity_mode);
>              ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>     lib/libavb/avb_cmdline.c:374:8: warning: 'verity_mode' may be used
>                                     uninitialized in this function
>                                     [-Wmaybe-uninitialized]
>        if (!cmdline_append_option(
>             ^~~~~~~~~~~~~~~~~~~~~~
>                slot_data, "androidboot.veritymode", verity_mode)) {
>                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> Warning #2:
> 
>     lib/libavb/avb_slot_verify.c: In function 'avb_slot_verify':
>     lib/libavb/avb_slot_verify.c:1349:23: warning: 'ret' may be used
>                                           uninitialized in this function
>                                           [-Wmaybe-uninitialized]
>        AvbSlotVerifyResult ret;
>                            ^~~

FWIW, few of Linux commits do word-wrapping of ASCII/console dumps for
the sake of improved readability and git grepping. Recent checkpatch
versions don't warn on that.

> 
> Fix those by providing default return values to affected functions.
> 
> Signed-off-by: Sam Protsenko <semen.protsenko at linaro.org>
> ---
>  lib/libavb/avb_cmdline.c     | 3 ++-
>  lib/libavb/avb_slot_verify.c | 2 +-
>  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/libavb/avb_cmdline.c b/lib/libavb/avb_cmdline.c
> index cb5b98e423..684c512bb9 100644
> --- a/lib/libavb/avb_cmdline.c
> +++ b/lib/libavb/avb_cmdline.c
> @@ -357,7 +357,8 @@ AvbSlotVerifyResult avb_append_options(
>          // Should never get here because MANAGED_RESTART_AND_EIO is
>          // remapped by avb_manage_hashtree_error_mode().
>          avb_assert_not_reached();
> -        break;
> +        ret = AVB_SLOT_VERIFY_RESULT_ERROR_INVALID_ARGUMENT;
> +        goto out;

With AVB_ENABLE_DEBUG enabled, PVS-Studio reports:
lib/libavb/avb_cmdline.c	360	warn	V779 Unreachable code detected. It is possible that an error is present.

How about replacing the 'break' statement with a 'fallthrough' comment?
It shuts down the warning w/o changing the functionality.

>        default:
>          ret = AVB_SLOT_VERIFY_RESULT_ERROR_INVALID_ARGUMENT;
>          goto out;
> diff --git a/lib/libavb/avb_slot_verify.c b/lib/libavb/avb_slot_verify.c
> index 5d400b38aa..c0defdf9c9 100644
> --- a/lib/libavb/avb_slot_verify.c
> +++ b/lib/libavb/avb_slot_verify.c
> @@ -1346,7 +1346,7 @@ AvbSlotVerifyResult avb_slot_verify(AvbOps* ops,
>                                      AvbSlotVerifyFlags flags,
>                                      AvbHashtreeErrorMode hashtree_error_mode,
>                                      AvbSlotVerifyData** out_data) {
> -  AvbSlotVerifyResult ret;
> +  AvbSlotVerifyResult ret = AVB_SLOT_VERIFY_RESULT_ERROR_INVALID_ARGUMENT;

What should we do with these out-of-tree libavb fixes? IMHO they are not
specific to U-Boot and should be upstream-able. IMHO it is not healthy
to accumulate too many of them, since this will make future libavb sync
more painful.

-- 
Best Regards,
Eugeniu.