diff mbox series

[1/5] iconvconfig: Make file handling more general purpose

Message ID 20210607085221.43612-2-siddhesh@sourceware.org
State Committed
Delegated to: DJ Delorie
Headers
Series gconv module configuration improvements |

Checks

Context Check Description
dj/TryBot-apply_patch success Patch applied to master at the time it was sent

Commit Message

Siddhesh Poyarekar June 7, 2021, 8:52 a.m. UTC 
  Split out configuration file handling code from handle_dir into its
own function so that it can be reused for multiple configuration
files.
---
 iconv/iconvconfig.c | 65 ++++++++++++++++++++++++++++-----------------
 1 file changed, 40 insertions(+), 25 deletions(-)

Comments

DJ Delorie June 8, 2021, 8:53 p.m. UTC | #1 
Siddhesh Poyarekar via Libc-alpha <libc-alpha@sourceware.org> writes:
> diff --git a/iconv/iconvconfig.c b/iconv/iconvconfig.c
> -
> -/* Read the config file and add the data for this directory to that.  */
> -static int
> -handle_dir (const char *dir)
> +/* Read a gconv-modules configuration file.  */
> +static bool
> +handle_file (const char *dir, const char *infile)

Rename existing function.  Keep "dir" as its used elsewhere in this
function, not shown by the patch.  Ok.

> -  char *cp;
>    FILE *fp;
>    char *line = NULL;
>    size_t linelen = 0;
> -  size_t dirlen = strlen (dir);
> -
> -  if (dir[dirlen - 1] != '/')
> -    {
> -      char *newp = (char *) xmalloc (dirlen + 2);
> -      dir = memcpy (newp, dir, dirlen);
> -      newp[dirlen++] = '/';
> -      newp[dirlen] = '\0';
> -    }
> -
> -  char infile[prefix_len + dirlen + sizeof "gconv-modules"];
> -  cp = infile;
> -  if (dir[0] == '/')
> -    cp = mempcpy (cp, prefix, prefix_len);
> -  strcpy (mempcpy (cp, dir, dirlen), "gconv-modules");

This portion is moved to handle_dir (below).  Ok.

>    fp = fopen (infile, "r");
>    if (fp == NULL)
> -    {
> -      error (0, errno, "cannot open `%s'", infile);
> -      return 1;
> -    }
> +    return false;

Error handling moved too, ok.

>    /* No threads present.  */
>    __fsetlocking (fp, FSETLOCKING_BYCALLER);
> @@ -723,7 +703,42 @@ handle_dir (const char *dir)
>  
>    fclose (fp);
>  
> -  return 0;
> +  return true;
> +}

Likewise.

> +/* Read config files and add the data for this directory to cache.  */
> +static int
> +handle_dir (const char *dir)
> +{
> +  char *cp;
> +  size_t dirlen = strlen (dir);
> +  bool found = false;
> +
> +  if (dir[dirlen - 1] != '/')
> +    {
> +      char *newp = (char *) xmalloc (dirlen + 2);
> +      dir = memcpy (newp, dir, dirlen);
> +      newp[dirlen++] = '/';
> +      newp[dirlen] = '\0';
> +    }
> +
> +  char infile[prefix_len + dirlen + sizeof "gconv-modules"];
> +  cp = infile;
> +  if (dir[0] == '/')
> +    cp = mempcpy (cp, prefix, prefix_len);
> +  strcpy (mempcpy (cp, dir, dirlen), "gconv-modules");
> +
> +  found |= handle_file (dir, infile);
> +
> +  if (!found)
> +    {
> +      error (0, errno, "failed to open gconv configuration file in `%s'",
> +	     dir);
> +      error (0, 0,
> +	     "ensure that the directory contains a valid gconv-modules file.");
> +    }
> +
> +  return found ? 0 : 1;
>  }

This looks a little weird as-is, but I realize further patches need it
this way, so OK.

Reviewed-by: DJ Delorie <dj@redhat.com>
diff mbox series

Patch

diff --git a/iconv/iconvconfig.c b/iconv/iconvconfig.c
index e04dd8a2e1..f5aba636d7 100644
--- a/iconv/iconvconfig.c
+++ b/iconv/iconvconfig.c
@@ -644,37 +644,17 @@  add_module (char *rp, const char *directory)
 	      cost, need_ext);
 }
 
-
-/* Read the config file and add the data for this directory to that.  */
-static int
-handle_dir (const char *dir)
+/* Read a gconv-modules configuration file.  */
+static bool
+handle_file (const char *dir, const char *infile)
 {
-  char *cp;
   FILE *fp;
   char *line = NULL;
   size_t linelen = 0;
-  size_t dirlen = strlen (dir);
-
-  if (dir[dirlen - 1] != '/')
-    {
-      char *newp = (char *) xmalloc (dirlen + 2);
-      dir = memcpy (newp, dir, dirlen);
-      newp[dirlen++] = '/';
-      newp[dirlen] = '\0';
-    }
-
-  char infile[prefix_len + dirlen + sizeof "gconv-modules"];
-  cp = infile;
-  if (dir[0] == '/')
-    cp = mempcpy (cp, prefix, prefix_len);
-  strcpy (mempcpy (cp, dir, dirlen), "gconv-modules");
 
   fp = fopen (infile, "r");
   if (fp == NULL)
-    {
-      error (0, errno, "cannot open `%s'", infile);
-      return 1;
-    }
+    return false;
 
   /* No threads present.  */
   __fsetlocking (fp, FSETLOCKING_BYCALLER);
@@ -723,7 +703,42 @@  handle_dir (const char *dir)
 
   fclose (fp);
 
-  return 0;
+  return true;
+}
+
+/* Read config files and add the data for this directory to cache.  */
+static int
+handle_dir (const char *dir)
+{
+  char *cp;
+  size_t dirlen = strlen (dir);
+  bool found = false;
+
+  if (dir[dirlen - 1] != '/')
+    {
+      char *newp = (char *) xmalloc (dirlen + 2);
+      dir = memcpy (newp, dir, dirlen);
+      newp[dirlen++] = '/';
+      newp[dirlen] = '\0';
+    }
+
+  char infile[prefix_len + dirlen + sizeof "gconv-modules"];
+  cp = infile;
+  if (dir[0] == '/')
+    cp = mempcpy (cp, prefix, prefix_len);
+  strcpy (mempcpy (cp, dir, dirlen), "gconv-modules");
+
+  found |= handle_file (dir, infile);
+
+  if (!found)
+    {
+      error (0, errno, "failed to open gconv configuration file in `%s'",
+	     dir);
+      error (0, 0,
+	     "ensure that the directory contains a valid gconv-modules file.");
+    }
+
+  return found ? 0 : 1;
 }