Hi, As for upstreaming libavb patches, I'd be interested in landing them upstream... makes it easier for anyone.
Our upstream is AOSP and we use gerrit for code-review: https://android-review.googlesource.com/q/project:platform%252Fexternal%252Favb Here's a guide to contributing: https://source.android.com/setup/contribute/submit-patches ... hope it's not too painful to use the AOSP process! Thanks, David On Fri, Aug 16, 2019 at 9:35 AM Eugeniu Rosca <ero...@de.adit-jv.com> wrote: > 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.protse...@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. > -- David Zeuthen | zeut...@google.com | Google | Android Hardware-Backed Security _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
