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

Message ID 20210607085221.43612-2-siddhesh@sourceware.org
State New
Headers show
Series
  • gconv module configuration improvements
Related show

Commit Message

Florian Weimer via Libc-alpha June 7, 2021, 8:52 a.m.
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(-)

-- 
2.31.1

Comments

Florian Weimer via Libc-alpha June 8, 2021, 8:53 p.m. | #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>

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;
 }