Skip to contentSkip to conversation

C23 string func() handling updates

PATCH In review Nicholas Vinson 2 participants 6 messages Feb 25 – Feb 26

lore
Share & tools
lore
#0 lore raw

v2 cover C23 string func() handling updates

With C23, the following 12 functions were updated to have 'const-aware' return types:

bsearch(), bsearch_s(), memchr(), strchr(), strpbrk(), strrchr(), strstr(), wcschr(), wcspbrk(), wcsrchr(), wcsstr(), and wmemchr().

Of these functions, strchr(), strpbrk(), strrchr(), and strstr(), are used within GRUB, and for 3 of these functions, strchr(), strrchr(), and strstr(), some of their calls within GRUB are now resulting in compile-time errors due to the C23 changes and 'discarding qualifier' errors. The errors are generated by elevating 'discarding qualifier' warnings to error-level. A promotion GRUB's build system defaults to doing.

Most of the changes this patch series makes to fix these errors are simple and only requiring changing a variable's type from 'char *' to 'const char *'. However, the changes in ofpath.c are more involved as they also update the way lengths are calculated and *printf() functions are called as a result of these C23 changes.

Furthermore, the changes are needed as C runtime libraries, notably glbc starting with version 2.43, are implementing the changes the C23 standard requires.

Nicholas Vinson (4):
grub-core/osdep/linux/ofpath.c: Update strstr() calls grub-core/osdep/linux/ofpath.c: correct path_size calculation util/probe.c: save strrchr() ret val to const data ptr. util/resolve.c: Save str[r]chr() ret val to const data ptr

 grub-core/osdep/linux/ofpath.c | 25 ++++++++++++-------------
 util/probe.c                   |  6 +++---
 util/resolve.c                 | 10 +++++-----
 3 files changed, 20 insertions(+), 21 deletions(-)
Signature · 7 lines
2.53.0


_______________________________________________
Grub-devel mailing list
[email protected]
https://lists.gnu.org/mailman/listinfo/grub-devel
#1 lore raw

PATCH v2 1/4 grub-core/osdep/linux/ofpath.c: Update strstr() calls

With C23, strstr() returns a 'const char *' if its first agrument is 'const char *', and these changes are implemented in glibc-2.43.

As a result, the first strstr() call in check_sas() now returns a 'const char *' instead of 'char *' because sysfs_path is a 'const char *'. This triggers a "discards qualifiers" warning, that is later promoted to an error and ultimately causes the build to fail.

To fix the issue, this patch converts 'ed', the pointer that stores strstr()'s return value, to a 'const char *'. Removes the xstrdup() call and cleans up the rest of the function by updating the *printf() calls and using pointer arthimetic to compute lengths instead of strlen().

---
 grub-core/osdep/linux/ofpath.c | 25 ++++++++++++-------------
 1 file changed, 12 insertions(+), 13 deletions(-)
grub-core/osdep/linux/ofpath.c +12 -13
index 24a4d5c8d..0ebfea44c 100644
@@ -488,8 +488,11 @@ check_hba_identifiers (const char *sysfs_path, int *vendor, int *device_id)
 static void
 check_sas (const char *sysfs_path, int *tgt, unsigned long int *sas_address)
 {
-  char *ed = strstr (sysfs_path, "end_device");
-  char *p, *q, *path;
+  const char *ed = strstr (sysfs_path, "end_device");
+  int p_len;
+  int ed_len;
+  const char *q;
+  char *path;
   char phy[21];
   int fd;
   size_t path_size;
@@ -498,20 +501,16 @@ check_sas (const char *sysfs_path, int *tgt, unsigned long int *sas_address)
     return;
 
   /* SAS devices are identified using disk@$PHY_ID */
-  p = xstrdup (sysfs_path);
-  ed = strstr(p, "end_device");
-  if (!ed)
-    return;
-
   q = ed;
   while (*q && *q != '/')
     q++;
-  *q = '\0';
+  p_len = (int) (q - sysfs_path);
+  ed_len = (int) (q - ed);
 
-  path_size = (strlen (p) + strlen (ed)
-	       + sizeof ("%s/sas_device/%s/phy_identifier"));
+  path_size = (p_len + ed_len + sizeof ("%s/sas_device/%s/phy_identifier"));
   path = xmalloc (path_size);
-  snprintf (path, path_size, "%s/sas_device/%s/phy_identifier", p, ed);
+  snprintf (path, path_size, "%.*s/sas_device/%.*s/phy_identifier", p_len,
+	    sysfs_path, ed_len, ed);
   fd = open (path, O_RDONLY);
   if (fd < 0)
     grub_util_error (_("cannot open `%s': %s"), path, strerror (errno));
@@ -524,7 +523,8 @@ check_sas (const char *sysfs_path, int *tgt, unsigned long int *sas_address)
 
   sscanf (phy, "%d", tgt);
 
-  snprintf (path, path_size, "%s/sas_device/%s/sas_address", p, ed);
+  snprintf (path, path_size, "%.*s/sas_device/%.*s/sas_address", p_len,
+	    sysfs_path, ed_len, ed);
   fd = open (path, O_RDONLY);
   if (fd < 0)
     grub_util_error (_("cannot open `%s': %s"), path, strerror (errno));
@@ -535,7 +535,6 @@ check_sas (const char *sysfs_path, int *tgt, unsigned long int *sas_address)
   sscanf (phy, "%lx", sas_address);
 
   free (path);
-  free (p);
   close (fd);
 }
 
Signature · 7 lines
2.53.0


_______________________________________________
Grub-devel mailing list
[email protected]
https://lists.gnu.org/mailman/listinfo/grub-devel
#2 lore raw

PATCH v2 2/4 grub-core/osdep/linux/ofpath.c: correct path_size calculation

In reply to Nicholas Vinson

path_size is computed in part by taking the size of the format string. However, the format string contains conversion specifiers. These conversion specifiers are included in the size calculation resulting in a size calculation that is larger than it needs to be. This patch corrects that error by removing the conversion specifiers when computing path_size.

---
 grub-core/osdep/linux/ofpath.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
grub-core/osdep/linux/ofpath.c +1 -1
index 0ebfea44c..38d9bb5bf 100644
@@ -507,7 +507,7 @@ check_sas (const char *sysfs_path, int *tgt, unsigned long int *sas_address)
   p_len = (int) (q - sysfs_path);
   ed_len = (int) (q - ed);
 
-  path_size = (p_len + ed_len + sizeof ("%s/sas_device/%s/phy_identifier"));
+  path_size = (p_len + ed_len + sizeof ("/sas_device//phy_identifier"));
   path = xmalloc (path_size);
   snprintf (path, path_size, "%.*s/sas_device/%.*s/phy_identifier", p_len,
 	    sysfs_path, ed_len, ed);
Signature · 7 lines
2.53.0


_______________________________________________
Grub-devel mailing list
[email protected]
https://lists.gnu.org/mailman/listinfo/grub-devel
#3 lore raw

PATCH v2 3/4 util/probe.c: save strrchr() ret val to const data ptr.

In reply to Nicholas Vinson

With glibc-2.43 implementing the C23 standard, strrchr() now returns 'const char *' when its first agrument is 'const char *'.

The fix is update all pointers receiving strrchr()'s return value so that they are now 'const char *' instead of 'char *'.

---
 util/probe.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
util/probe.c +3 -3
index 81d91cf59..a9158d640 100644
@@ -70,7 +70,7 @@ char *
 grub_util_guess_bios_drive (const char *orig_path)
 {
   char *canon;
-  char *ptr;
+  const char *ptr;
   canon = grub_canonicalize_file_name (orig_path);
   if (!canon)
     return NULL;
@@ -99,7 +99,7 @@ char *
 grub_util_guess_efi_drive (const char *orig_path)
 {
   char *canon;
-  char *ptr;
+  const char *ptr;
   canon = grub_canonicalize_file_name (orig_path);
   if (!canon)
     return NULL;
@@ -128,7 +128,7 @@ char *
 grub_util_guess_baremetal_drive (const char *orig_path)
 {
   char *canon;
-  char *ptr;
+  const char *ptr;
   canon = grub_canonicalize_file_name (orig_path);
   if (!canon)
     return NULL;
Signature · 7 lines
2.53.0


_______________________________________________
Grub-devel mailing list
[email protected]
https://lists.gnu.org/mailman/listinfo/grub-devel
#4 lore raw

PATCH v2 4/4 util/resolve.c: Save str[r]chr() ret val to const data ptr

In reply to Nicholas Vinson

With glibc-2.43 implementing the C23 standard, strrchr() and strchr() now return 'const char *' when its first agrument is 'const char *'.

The fix is update all pointers receiving strrchr() and strchr()'s return values so that they are now 'const char *' instead of 'char *'.

---
 util/resolve.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)
util/resolve.c +5 -5
index b6e26312f..254379195 100644
@@ -138,12 +138,12 @@ read_dep_list (FILE *fp)
 static char *
 get_module_name (const char *str)
 {
-  char *base;
-  char *ext;
+  const char *base;
+  const char *ext;
 
   base = strrchr (str, '/');
   if (! base)
-    base = (char *) str;
+    base = str;
   else
     base++;
 
@@ -164,9 +164,9 @@ get_module_name (const char *str)
 static char *
 get_module_path (const char *prefix, const char *str)
 {
-  char *dir;
+  const char *dir;
   char *base;
-  char *ext;
+  const char *ext;
   char *ret;
 
   ext = strrchr (str, '.');
Signature · 7 lines
2.53.0


_______________________________________________
Grub-devel mailing list
[email protected]
https://lists.gnu.org/mailman/listinfo/grub-devel
#5 lore raw
In reply to Nicholas Vinson
Quoted 24 lines · Nicholas Vinson

With C23, the following 12 functions were updated to have 'const-aware' return types:

bsearch(), bsearch_s(), memchr(), strchr(), strpbrk(), strrchr(), strstr(), wcschr(), wcspbrk(), wcsrchr(), wcsstr(), and wmemchr().

Of these functions, strchr(), strpbrk(), strrchr(), and strstr(), are used within GRUB, and for 3 of these functions, strchr(), strrchr(), and strstr(), some of their calls within GRUB are now resulting in compile-time errors due to the C23 changes and 'discarding qualifier' errors. The errors are generated by elevating 'discarding qualifier' warnings to error-level. A promotion GRUB's build system defaults to doing.

Most of the changes this patch series makes to fix these errors are simple and only requiring changing a variable's type from 'char *' to 'const char *'. However, the changes in ofpath.c are more involved as they also update the way lengths are calculated and *printf() functions are called as a result of these C23 changes.

Furthermore, the changes are needed as C runtime libraries, notably glbc starting with version 2.43, are implementing the changes the C23 standard requires.

Quoted 7 lines

Nicholas Vinson (4):
grub-core/osdep/linux/ofpath.c: Update strstr() calls grub-core/osdep/linux/ofpath.c: correct path_size calculation util/probe.c: save strrchr() ret val to const data ptr. util/resolve.c: Save str[r]chr() ret val to const data ptr

For all patches Reviewed-by: Daniel Kiper <[email protected]>...

Thank you for fixing these issues!

Daniel

_______________________________________________
Grub-devel mailing list
[email protected]
https://lists.gnu.org/mailman/listinfo/grub-devel