On 07.12.23 13:28, 'Jan Kiszka' via EFI Boot Guard wrote:
> On 07.12.23 11:37, 'Michael Adler' via EFI Boot Guard wrote:
>> This commit changes the default verbosity setting for the FAT parser to
>> false, aligning it with the general efibootguard behavior where
>> verbosity is not enabled by default. Users can still opt-in for verbose
>> output by using the -v or --verbose flag.
>>
>
> This affects the output of tools only, right?
>
> And is this a regression fix or a cleanup of an inconsistency?
>
>> Signed-off-by: Michael Adler <michael.ad...@siemens.com>
>> ---
>> tools/ebgpart.c | 2 +-
>> tools/fat.c | 6 +++---
>> tools/fat.h | 2 +-
>> tools/tests/test_fat.c | 21 +++++++++++++--------
>> 4 files changed, 18 insertions(+), 13 deletions(-)
>>
>> diff --git a/tools/ebgpart.c b/tools/ebgpart.c
>> index b6384f2..298d766 100644
>> --- a/tools/ebgpart.c
>> +++ b/tools/ebgpart.c
>> @@ -124,7 +124,7 @@ static int check_GPT_FAT_entry(int fd, const struct
>> EFIpartitionentry *e)
>> strerror(errno));
>> return -1;
>> }
>> - return determine_FAT_bits(&header);
>> + return determine_FAT_bits(&header, verbosity);
>> }
>>
>> static inline EbgFileSystemType fat_size_to_fs_type(int fat_size)
>> diff --git a/tools/fat.c b/tools/fat.c
>> index 0ff0f64..cc9d9ff 100644
>> --- a/tools/fat.c
>> +++ b/tools/fat.c
>> @@ -22,8 +22,6 @@
>> #include "linux_util.h"
>> #include "ebgpart.h"
>>
>> -static bool verbosity = true;
>> -
>> #define fat_msg(sb, lvl, ...) do { VERBOSE(stderr, __VA_ARGS__);
>> VERBOSE(stderr, "\n"); } while(0);
>> #define KERN_ERR "ERROR"
>>
>> @@ -69,6 +67,8 @@ static int fat_read_bpb(void __attribute__((unused)) *sb,
>> const struct fat_boot_sector *b, int silent,
>> struct fat_bios_param_block *bpb)
>> {
>> + bool verbosity = !silent; /* for our VERBOSE macro */
>> +
>> int error = -EINVAL;
>>
>> /* Read in BPB ... */
>> @@ -147,7 +147,7 @@ out:
>> /* end of Linux kernel code */
>>
>> /*****************************************************************************/
>>
>> -int determine_FAT_bits(const struct fat_boot_sector *sector)
>> +int determine_FAT_bits(const struct fat_boot_sector *sector, bool verbosity)
>> {
>> struct fat_bios_param_block bpb;
>> if (fat_read_bpb(NULL, sector, !verbosity, &bpb)) {
>> diff --git a/tools/fat.h b/tools/fat.h
>> index 8c991ab..d0cf3c7 100644
>> --- a/tools/fat.h
>> +++ b/tools/fat.h
>> @@ -22,4 +22,4 @@
>> * to ensure it is a valid FAT boot sector. If the provided boot sector is
>> not valid or an error
>> * occurs during the determination process, the function returns a value
>> less than or equal to 0.
>> */
>> -int determine_FAT_bits(const struct fat_boot_sector *sector);
>> +int determine_FAT_bits(const struct fat_boot_sector *sector, bool
>> verbosity);
>> diff --git a/tools/tests/test_fat.c b/tools/tests/test_fat.c
>> index 2c5089f..42e0133 100644
>> --- a/tools/tests/test_fat.c
>> +++ b/tools/tests/test_fat.c
>> @@ -34,7 +34,7 @@ START_TEST(test_determine_FAT_bits_empty)
>> {
>> struct fat_boot_sector sector;
>> memset(§or, 0, sizeof(sector));
>> - int ret = determine_FAT_bits(§or);
>> + int ret = determine_FAT_bits(§or, true);
>> ck_assert_int_eq(ret, 0);
>> }
>> END_TEST
>> @@ -48,7 +48,7 @@ START_TEST(test_determine_FAT_bits_sec_per_clus_zero)
>> .media = 0xf8,
>> };
>> u16_to_le(512, sector.sector_size);
>> - int ret = determine_FAT_bits(§or);
>> + int ret = determine_FAT_bits(§or, true);
>> ck_assert_int_eq(ret, 0);
>> }
>> END_TEST
>> @@ -62,7 +62,7 @@ START_TEST(test_determine_FAT_bits_fat_sector_size_zero)
>> .fats = 16,
>> .media = 0xf8,
>> };
>> - int ret = determine_FAT_bits(§or);
>> + int ret = determine_FAT_bits(§or, true);
>> ck_assert_int_eq(ret, 0);
>> }
>> END_TEST
>> @@ -124,7 +124,8 @@ START_TEST(test_determine_FAT_bits_12)
>> 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
>> 0x55, 0xaa,
>> };
>> - int ret = determine_FAT_bits((const struct fat_boot_sector*) §or);
>> + int ret = determine_FAT_bits((const struct fat_boot_sector *)§or,
>> + true);
>> ck_assert_int_eq(ret, 12);
>> }
>> END_TEST
>> @@ -186,7 +187,8 @@ START_TEST(test_determine_FAT_bits_16)
>> 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
>> 0x55, 0xaa,
>> };
>> - int ret = determine_FAT_bits((const struct fat_boot_sector*) §or);
>> + int ret = determine_FAT_bits((const struct fat_boot_sector *)§or,
>> + true);
>> ck_assert_int_eq(ret, 16);
>> }
>> END_TEST
>> @@ -248,7 +250,8 @@ START_TEST(test_determine_FAT_bits_32)
>> 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
>> 0x55, 0xaa,
>> };
>> - int ret = determine_FAT_bits((const struct fat_boot_sector*) §or);
>> + int ret = determine_FAT_bits((const struct fat_boot_sector *)§or,
>> + true);
>> ck_assert_int_eq(ret, 32);
>> }
>> END_TEST
>> @@ -312,7 +315,8 @@ START_TEST(test_determine_FAT_bits_fat16_swupdate)
>> 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
>> 0x55, 0xaa,
>> };
>> - int ret = determine_FAT_bits((const struct fat_boot_sector*) §or);
>> + int ret = determine_FAT_bits((const struct fat_boot_sector *)§or,
>> + true);
>> ck_assert_int_eq(ret, 16);
>> }
>> END_TEST
>> @@ -373,7 +377,8 @@ START_TEST(test_determine_FAT_bits_squashfs)
>> 0xd7, 0x40, 0xc1, 0xcc, 0x43, 0x7b, 0xbf, 0x8d, 0x76, 0x39,
>> 0x3c, 0xd5,
>> };
>> - int ret = determine_FAT_bits((const struct fat_boot_sector *)§or);
>> + int ret = determine_FAT_bits((const struct fat_boot_sector *)§or,
>> + true);
>> ck_assert_int_eq(ret, 0);
>> }
>> END_TEST
>
>
> Looks good otherwise.
>
...but not to scripts/cppcheck.sh. Please cross-check as this would
likely also explode in CI:
tools/fat.c:97:4: warning: Identical inner 'if' condition is always true.
[identicalInnerCondition]
fat_msg(sb, KERN_ERR,
^
tools/fat.c:70:19: note: 'verbosity' is assigned value '!silent' here.
bool verbosity = !silent; /* for our VERBOSE macro */
^
tools/fat.c:96:7: note: outer condition: !silent
if (!silent)
^
tools/fat.c:97:4: note: identical inner condition: verbosity
fat_msg(sb, KERN_ERR,
^
tools/fat.c:103:4: warning: Identical inner 'if' condition is always true.
[identicalInnerCondition]
fat_msg(sb, KERN_ERR, "bogus number of FAT structure");
^
tools/fat.c:70:19: note: 'verbosity' is assigned value '!silent' here.
bool verbosity = !silent; /* for our VERBOSE macro */
^
tools/fat.c:102:7: note: outer condition: !silent
if (!silent)
^
tools/fat.c:103:4: note: identical inner condition: verbosity
fat_msg(sb, KERN_ERR, "bogus number of FAT structure");
^
tools/fat.c:114:4: warning: Identical inner 'if' condition is always true.
[identicalInnerCondition]
fat_msg(sb, KERN_ERR, "invalid media value (0x%02x)",
^
tools/fat.c:70:19: note: 'verbosity' is assigned value '!silent' here.
bool verbosity = !silent; /* for our VERBOSE macro */
^
tools/fat.c:113:7: note: outer condition: !silent
if (!silent)
^
tools/fat.c:114:4: note: identical inner condition: verbosity
fat_msg(sb, KERN_ERR, "invalid media value (0x%02x)",
^
tools/fat.c:123:4: warning: Identical inner 'if' condition is always true.
[identicalInnerCondition]
fat_msg(sb, KERN_ERR, "bogus logical sector size %u",
^
tools/fat.c:70:19: note: 'verbosity' is assigned value '!silent' here.
bool verbosity = !silent; /* for our VERBOSE macro */
^
tools/fat.c:122:7: note: outer condition: !silent
if (!silent)
^
tools/fat.c:123:4: note: identical inner condition: verbosity
fat_msg(sb, KERN_ERR, "bogus logical sector size %u",
^
tools/fat.c:130:4: warning: Identical inner 'if' condition is always true.
[identicalInnerCondition]
fat_msg(sb, KERN_ERR, "bogus sectors per cluster %u",
^
tools/fat.c:70:19: note: 'verbosity' is assigned value '!silent' here.
bool verbosity = !silent; /* for our VERBOSE macro */
^
tools/fat.c:129:7: note: outer condition: !silent
if (!silent)
^
tools/fat.c:130:4: note: identical inner condition: verbosity
fat_msg(sb, KERN_ERR, "bogus sectors per cluster %u",
^
tools/fat.c:137:4: warning: Identical inner 'if' condition is always true.
[identicalInnerCondition]
fat_msg(sb, KERN_ERR, "bogus number of FAT sectors");
^
tools/fat.c:70:19: note: 'verbosity' is assigned value '!silent' here.
bool verbosity = !silent; /* for our VERBOSE macro */
^
tools/fat.c:136:7: note: outer condition: !silent
if (!silent)
^
tools/fat.c:137:4: note: identical inner condition: verbosity
fat_msg(sb, KERN_ERR, "bogus number of FAT sectors");
^
Jan
--
Siemens AG, Technology
Linux Expert Center
--
You received this message because you are subscribed to the Google Groups "EFI
Boot Guard" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to efibootguard-dev+unsubscr...@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/efibootguard-dev/ed009cd0-6922-4e2f-8c87-e688cc6db4a4%40siemens.com.
