On 07/12/2017 12:08 PM, [ext] Andreas Reichel wrote:


On 07/11/2017 08:44 PM, Claudius Heine wrote:
On Tue, 2017-07-11 at 15:37 +0200, Reichel Andreas wrote:
* Allow FAT12, FAT16 and FAT32 for environment data.
* Minor code style fixes

Signed-off-by: Andreas Reichel <[email protected]>
---
  tools/bg_utils.c | 13 +++++++++----
  1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/tools/bg_utils.c b/tools/bg_utils.c
index 36ee279..257d8be 100644
--- a/tools/bg_utils.c
+++ b/tools/bg_utils.c
@@ -151,7 +151,7 @@ static FILE *open_config_file(CONFIG_PART
*cfgpart, char *mode)
      strncat(configfilepath, "/", 1);
      strncat(configfilepath, FAT_ENV_FILENAME,
strlen(FAT_ENV_FILENAME));
      VERBOSE(stdout, "Probing config file at %s.\n",
configfilepath);
-    FILE* config = fopen(configfilepath, mode);
+    FILE *config = fopen(configfilepath, mode);
      free(configfilepath);
      return config;
  }
@@ -224,7 +224,9 @@ bool probe_config_partitions(CONFIG_PART
*cfgpart)
          PedPartition *part = pd->part_list;
          while (part) {
              if (!part->fs_type || !part->fs_type->name
||
-                strcmp(part->fs_type->name, "fat16") !=
0) {
+                (strcmp(part->fs_type->name, "fat12") !=
0 &&
+                 strcmp(part->fs_type->name, "fat16") !=
0 &&
+                 strcmp(part->fs_type->name, "fat32") != 0)) {
Are there any fat* left?
You could just use strncmp(part->fs_type->name, "fat", sizeof(char)*3);
This is an enumeration over all allowed file system types. Just
comparing the first three letters is not a robust and future oriented
way. Assume somebody will introduce FATXY or alike... then this can easily
break.

Wouldn't you need to write code to set fs_type->name to the new and shiny "FATXY" first?

Then maybe use fnmatch("fat[13][26]", part->fs_type->name, 0) and hope that nobody creates a fat36 ;)

Ok leave it like that if you really want. I just don't like seeing this kind of replicated code knowing the the strings are iterated over 3 times in a row while comparing them to very similar variations. But that could just be premature optimization.

                  part = ped_disk_next_partition(pd,
part);
                  continue;
              }
@@ -450,11 +452,14 @@ bool bgenv_write(BGENV *env)
      case BGENVTYPE_FAT:
          part = (CONFIG_PART *)env->desc;
          if (!part) {
-            VERBOSE(stderr, "Invalid config partition to
store environment.\n");
+            VERBOSE(
+                stderr,
+                "Invalid config partition to store
environment.\n");
              return false;
          }
          if (!write_env(part, env->data)) {
-            VERBOSE(stderr, "Could not write to %s\n",
part->devpath);
+            VERBOSE(stderr, "Could not write to %s\n",
+                part->devpath);
              return false;
          }
          return true;
--
2.11.0



--
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 [email protected].
To post to this group, send email to [email protected].
To view this discussion on the web visit 
https://groups.google.com/d/msgid/efibootguard-dev/72f190e7-71ec-eba1-983d-6a3df9a46b97%40siemens.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to