[3/6] gconv_conf: Split out configuration file processing

Message ID 20210610111853.2286873-4-siddhesh@sourceware.org
State New
Headers show
Series
  • gconv configuration parsing cleanups
Related show

Commit Message

Florian Weimer via Libc-alpha June 10, 2021, 11:18 a.m.
Split configuration file processing into a separate header file and
include it.  Macroize all calls that need to go through internal
interfaces so that iconvconfig can also use them.
---
 iconv/gconv_conf.c         | 129 +----------------------------
 iconv/gconv_parseconfdir.h | 161 +++++++++++++++++++++++++++++++++++++
 2 files changed, 164 insertions(+), 126 deletions(-)
 create mode 100644 iconv/gconv_parseconfdir.h

-- 
2.31.1

Comments

Florian Weimer via Libc-alpha June 22, 2021, 3:48 a.m. | #1
Siddhesh Poyarekar <siddhesh@sourceware.org> writes:
> Split configuration file processing into a separate header file and

> include it.  Macroize all calls that need to go through internal

> interfaces so that iconvconfig can also use them.


> -#include <dirent.h>

>  #include <errno.h>

>  #include <limits.h>

>  #include <locale.h>

> @@ -31,11 +30,10 @@

>  #include <string.h>

>  #include <unistd.h>

>  #include <sys/param.h>

> -#include <sys/types.h>

>  

>  #include <libc-lock.h>

>  #include <gconv_int.h>

> -

> +#include <gconv_parseconfdir.h>


Ok; gconv_parseconfdir.h includes those two deleted ones.

> -/* Name of the file containing the module information in the directories

> -   along the path.  */

> -static const char gconv_conf_filename[] = "gconv-modules";

> -static const char gconv_conf_dirname[] = "gconv-modules.d";

> -


Moved to .h; ok.

> -#include <libio/libioP.h>

> -#define __getdelim(line, len, c, fp) _IO_getdelim (line, len, c, fp)

> -


Moved, ok.

> -/* Read the next configuration file.  */

> -static void

> -read_conf_file (const char *filename, const char *directory, size_t dir_len)

> -{

> -  /* Note the file is opened with cancellation in the I/O functions

> -     disabled.  */

> -  FILE *fp = fopen (filename, "rce");

> -  char *line = NULL;

> -  size_t line_len = 0;

> -  static int modcounter;

> -

> -  /* Don't complain if a file is not present or readable, simply silently

> -     ignore it.  */

> -  if (fp == NULL)

> -    return;

> -

> -  /* No threads reading from this stream.  */

> -  __fsetlocking (fp, FSETLOCKING_BYCALLER);

> -

> -  /* Process the known entries of the file.  Comments start with `#' and

> -     end with the end of the line.  Empty lines are ignored.  */

> -  while (!__feof_unlocked (fp))

> -    {

> -      char *rp, *endp, *word;

> -      ssize_t n = __getdelim (&line, &line_len, '\n', fp);

> -      if (n < 0)

> -	/* An error occurred.  */

> -	break;

> -

> -      rp = line;

> -      /* Terminate the line (excluding comments or newline) by an NUL byte

> -	 to simplify the following code.  */

> -      endp = strchr (rp, '#');

> -      if (endp != NULL)

> -	*endp = '\0';

> -      else

> -	if (rp[n - 1] == '\n')

> -	  rp[n - 1] = '\0';

> -

> -      while (__isspace_l (*rp, _nl_C_locobj_ptr))

> -	++rp;

> -

> -      /* If this is an empty line go on with the next one.  */

> -      if (rp == endp)

> -	continue;

> -

> -      word = rp;

> -      while (*rp != '\0' && !__isspace_l (*rp, _nl_C_locobj_ptr))

> -	++rp;

> -

> -      if (rp - word == sizeof ("alias") - 1

> -	  && memcmp (word, "alias", sizeof ("alias") - 1) == 0)

> -	add_alias (rp);

> -      else if (rp - word == sizeof ("module") - 1

> -	       && memcmp (word, "module", sizeof ("module") - 1) == 0)

> -	add_module (rp, directory, dir_len, modcounter++);

> -      /* else */

> -	/* Otherwise ignore the line.  */

> -    }

> -

> -  free (line);

> -

> -  fclose (fp);

> -}

> -

> -


Moved to .h; ok.

> @@ -554,55 +478,8 @@ __gconv_read_conf (void)

>    __gconv_get_path ();

>  

>    for (cnt = 0; __gconv_path_elem[cnt].name != NULL; ++cnt)

> -    {

> -      const char *elem = __gconv_path_elem[cnt].name;

> -      size_t elem_len = __gconv_path_elem[cnt].len;

> -

> -      /* No slash needs to be inserted between elem and gconv_conf_filename;

> -	 elem already ends in a slash.  */

> -      char *buf = malloc (elem_len + sizeof (gconv_conf_dirname));

> -      if (buf == NULL)

> -	continue;

> -

> -      char *cp = __mempcpy (__mempcpy (buf, elem, elem_len),

> -			    gconv_conf_filename, sizeof (gconv_conf_filename));

> -

> -      /* Read the gconv-modules configuration file first.  */

> -      read_conf_file (buf, elem, elem_len);

> -

> -      /* Next, see if there is a gconv-modules.d directory containing

> -	 configuration files and if it is non-empty.  */

> -      cp--;

> -      cp[0] = '.';

> -      cp[1] = 'd';

> -      cp[2] = '\0';

> -

> -      DIR *confdir = __opendir (buf);

> -      if (confdir != NULL)

> -	{

> -	  struct dirent *ent;

> -	  while ((ent = __readdir (confdir)) != NULL)

> -	    {

> -	      if (ent->d_type != DT_REG)

> -		continue;

> -

> -	      size_t len = strlen (ent->d_name);

> -	      const char *suffix = ".conf";

> -

> -	      if (len > strlen (suffix)

> -		  && strcmp (ent->d_name + len - strlen (suffix), suffix) == 0)

> -		{

> -		  char *conf;

> -		  if (__asprintf (&conf, "%s/%s", buf, ent->d_name) < 0)

> -		    continue;

> -		  read_conf_file (conf, elem, elem_len);

> -		  free (conf);

> -		}

> -	    }

> -	  __closedir (confdir);

> -	}

> -      free (buf);

> -    }

> +    gconv_parseconfdir (__gconv_path_elem[cnt].name,

> +			__gconv_path_elem[cnt].len);

>  #endif


Ok, found in .h.

> diff --git a/iconv/gconv_parseconfdir.h b/iconv/gconv_parseconfdir.h

> new file mode 100644

> index 0000000000..3d4d58d4be

> --- /dev/null

> +++ b/iconv/gconv_parseconfdir.h

> @@ -0,0 +1,161 @@

> +/* Handle configuration data.

> +   Copyright (C) 2021 Free Software Foundation, Inc.

> +   This file is part of the GNU C Library.

> +

> +   The GNU C Library is free software; you can redistribute it and/or

> +   modify it under the terms of the GNU Lesser General Public

> +   License as published by the Free Software Foundation; either

> +   version 2.1 of the License, or (at your option) any later version.

> +

> +   The GNU C Library is distributed in the hope that it will be useful,

> +   but WITHOUT ANY WARRANTY; without even the implied warranty of

> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU

> +   Lesser General Public License for more details.

> +

> +   You should have received a copy of the GNU Lesser General Public

> +   License along with the GNU C Library; if not, see

> +   <https://www.gnu.org/licenses/>.  */


Ok.

> +#include <dirent.h>

> +#include <libc-symbols.h>

> +#include <locale.h>

> +#include <sys/types.h>


Ok.

> +#if IS_IN (libc)

> +# include <libio/libioP.h>

> +# define __getdelim(line, len, c, fp) _IO_getdelim (line, len, c, fp)

> +

> +# undef isspace

> +# define isspace(__c) __isspace_l ((__c), _nl_C_locobj_ptr)

> +# define asprintf __asprintf

> +# define opendir __opendir

> +# define readdir __readdir

> +# define closedir __closedir

> +# define mempcpy __mempcpy

> +#endif


Ok.

> +/* Name of the file containing the module information in the directories

> +   along the path.  */

> +static const char gconv_conf_filename[] = "gconv-modules";

> +static const char gconv_conf_dirname[] = "gconv-modules.d";


Ok.

> +static void add_alias (char *);

> +static void add_module (char *, const char *, size_t, int);


Ok.

> +/* Read the next configuration file.  */

> +static bool

> +read_conf_file (const char *filename, const char *directory, size_t dir_len)

> +{

> +  /* Note the file is opened with cancellation in the I/O functions

> +     disabled.  */

> +  FILE *fp = fopen (filename, "rce");

> +  char *line = NULL;

> +  size_t line_len = 0;

> +  static int modcounter;

> +

> +  /* Don't complain if a file is not present or readable, simply silently

> +     ignore it.  */

> +  if (fp == NULL)

> +    return false;

> +

> +  /* No threads reading from this stream.  */

> +  __fsetlocking (fp, FSETLOCKING_BYCALLER);

> +

> +  /* Process the known entries of the file.  Comments start with `#' and

> +     end with the end of the line.  Empty lines are ignored.  */

> +  while (!__feof_unlocked (fp))

> +    {

> +      char *rp, *endp, *word;

> +      ssize_t n = __getdelim (&line, &line_len, '\n', fp);

> +      if (n < 0)

> +	/* An error occurred.  */

> +	break;

> +

> +      rp = line;

> +      /* Terminate the line (excluding comments or newline) by an NUL byte

> +	 to simplify the following code.  */

> +      endp = strchr (rp, '#');

> +      if (endp != NULL)

> +	*endp = '\0';

> +      else

> +	if (rp[n - 1] == '\n')

> +	  rp[n - 1] = '\0';

> +

> +      while (isspace (*rp))

> +	++rp;

> +

> +      /* If this is an empty line go on with the next one.  */

> +      if (rp == endp)

> +	continue;

> +

> +      word = rp;

> +      while (*rp != '\0' && !isspace (*rp))

> +	++rp;

> +

> +      if (rp - word == sizeof ("alias") - 1

> +	  && memcmp (word, "alias", sizeof ("alias") - 1) == 0)

> +	add_alias (rp);

> +      else if (rp - word == sizeof ("module") - 1

> +	       && memcmp (word, "module", sizeof ("module") - 1) == 0)

> +	add_module (rp, directory, dir_len, modcounter++);

> +      /* else */

> +	/* Otherwise ignore the line.  */

> +    }

> +

> +  free (line);

> +

> +  fclose (fp);

> +  return true;

> +}

> +


Copied from above; ok.

> +static __always_inline bool

> +gconv_parseconfdir (const char *dir, size_t dir_len)

> +{

> +  /* No slash needs to be inserted between dir and gconv_conf_filename;

> +     dir already ends in a slash.  */

> +  char *buf = malloc (dir_len + sizeof (gconv_conf_dirname));

> +  bool found = false;

> +

> +  if (buf == NULL)

> +    return false;

> +

> +  char *cp = mempcpy (mempcpy (buf, dir, dir_len), gconv_conf_filename,

> +		      sizeof (gconv_conf_filename));

> +

> +  /* Read the gconv-modules configuration file first.  */

> +  found = read_conf_file (buf, dir, dir_len);

> +

> +  /* Next, see if there is a gconv-modules.d directory containing

> +     configuration files and if it is non-empty.  */

> +  cp--;

> +  cp[0] = '.';

> +  cp[1] = 'd';

> +  cp[2] = '\0';

> +

> +  DIR *confdir = opendir (buf);

> +  if (confdir != NULL)

> +    {

> +      struct dirent *ent;

> +      while ((ent = readdir (confdir)) != NULL)

> +	{

> +	  if (ent->d_type != DT_REG)

> +	    continue;

> +

> +	  size_t len = strlen (ent->d_name);

> +	  const char *suffix = ".conf";

> +

> +	  if (len > strlen (suffix)

> +	      && strcmp (ent->d_name + len - strlen (suffix), suffix) == 0)

> +	    {

> +	      char *conf;

> +	      if (asprintf (&conf, "%s/%s", buf, ent->d_name) < 0)

> +		continue;

> +	      found |= read_conf_file (conf, dir, dir_len);

> +	      free (conf);

> +	    }

> +	}

> +      closedir (confdir);

> +    }

> +  free (buf);

> +  return found;

> +}


Extracted from above; ok.

LGTM.
Reviewed-by: DJ Delorie <dj@redhat.com>

Patch

diff --git a/iconv/gconv_conf.c b/iconv/gconv_conf.c
index 6c6625c37a..62bee28769 100644
--- a/iconv/gconv_conf.c
+++ b/iconv/gconv_conf.c
@@ -19,7 +19,6 @@ 
 
 #include <assert.h>
 #include <ctype.h>
-#include <dirent.h>
 #include <errno.h>
 #include <limits.h>
 #include <locale.h>
@@ -31,11 +30,10 @@ 
 #include <string.h>
 #include <unistd.h>
 #include <sys/param.h>
-#include <sys/types.h>
 
 #include <libc-lock.h>
 #include <gconv_int.h>
-
+#include <gconv_parseconfdir.h>
 
 /* This is the default path where we look for module lists.  */
 static const char default_gconv_path[] = GCONV_PATH;
@@ -56,11 +54,6 @@  size_t __gconv_max_path_elem_len;
 /* We use the following struct if we couldn't allocate memory.  */
 static const struct path_elem empty_path_elem = { NULL, 0 };
 
-/* Name of the file containing the module information in the directories
-   along the path.  */
-static const char gconv_conf_filename[] = "gconv-modules";
-static const char gconv_conf_dirname[] = "gconv-modules.d";
-
 /* Filename extension for the modules.  */
 #ifndef MODULE_EXT
 # define MODULE_EXT ".so"
@@ -99,9 +92,6 @@  static const char builtin_aliases[] =
 #undef BUILTIN_ALIAS
 };
 
-#include <libio/libioP.h>
-#define __getdelim(line, len, c, fp) _IO_getdelim (line, len, c, fp)
-
 
 /* Value of the GCONV_PATH environment variable.  */
 const char *__gconv_path_envvar;
@@ -361,72 +351,6 @@  add_module (char *rp, const char *directory, size_t dir_len, int modcounter)
 }
 
 
-/* Read the next configuration file.  */
-static void
-read_conf_file (const char *filename, const char *directory, size_t dir_len)
-{
-  /* Note the file is opened with cancellation in the I/O functions
-     disabled.  */
-  FILE *fp = fopen (filename, "rce");
-  char *line = NULL;
-  size_t line_len = 0;
-  static int modcounter;
-
-  /* Don't complain if a file is not present or readable, simply silently
-     ignore it.  */
-  if (fp == NULL)
-    return;
-
-  /* No threads reading from this stream.  */
-  __fsetlocking (fp, FSETLOCKING_BYCALLER);
-
-  /* Process the known entries of the file.  Comments start with `#' and
-     end with the end of the line.  Empty lines are ignored.  */
-  while (!__feof_unlocked (fp))
-    {
-      char *rp, *endp, *word;
-      ssize_t n = __getdelim (&line, &line_len, '\n', fp);
-      if (n < 0)
-	/* An error occurred.  */
-	break;
-
-      rp = line;
-      /* Terminate the line (excluding comments or newline) by an NUL byte
-	 to simplify the following code.  */
-      endp = strchr (rp, '#');
-      if (endp != NULL)
-	*endp = '\0';
-      else
-	if (rp[n - 1] == '\n')
-	  rp[n - 1] = '\0';
-
-      while (__isspace_l (*rp, _nl_C_locobj_ptr))
-	++rp;
-
-      /* If this is an empty line go on with the next one.  */
-      if (rp == endp)
-	continue;
-
-      word = rp;
-      while (*rp != '\0' && !__isspace_l (*rp, _nl_C_locobj_ptr))
-	++rp;
-
-      if (rp - word == sizeof ("alias") - 1
-	  && memcmp (word, "alias", sizeof ("alias") - 1) == 0)
-	add_alias (rp);
-      else if (rp - word == sizeof ("module") - 1
-	       && memcmp (word, "module", sizeof ("module") - 1) == 0)
-	add_module (rp, directory, dir_len, modcounter++);
-      /* else */
-	/* Otherwise ignore the line.  */
-    }
-
-  free (line);
-
-  fclose (fp);
-}
-
-
 /* Determine the directories we are looking for data in.  This function should
    only be called from __gconv_read_conf.  */
 static void
@@ -554,55 +478,8 @@  __gconv_read_conf (void)
   __gconv_get_path ();
 
   for (cnt = 0; __gconv_path_elem[cnt].name != NULL; ++cnt)
-    {
-      const char *elem = __gconv_path_elem[cnt].name;
-      size_t elem_len = __gconv_path_elem[cnt].len;
-
-      /* No slash needs to be inserted between elem and gconv_conf_filename;
-	 elem already ends in a slash.  */
-      char *buf = malloc (elem_len + sizeof (gconv_conf_dirname));
-      if (buf == NULL)
-	continue;
-
-      char *cp = __mempcpy (__mempcpy (buf, elem, elem_len),
-			    gconv_conf_filename, sizeof (gconv_conf_filename));
-
-      /* Read the gconv-modules configuration file first.  */
-      read_conf_file (buf, elem, elem_len);
-
-      /* Next, see if there is a gconv-modules.d directory containing
-	 configuration files and if it is non-empty.  */
-      cp--;
-      cp[0] = '.';
-      cp[1] = 'd';
-      cp[2] = '\0';
-
-      DIR *confdir = __opendir (buf);
-      if (confdir != NULL)
-	{
-	  struct dirent *ent;
-	  while ((ent = __readdir (confdir)) != NULL)
-	    {
-	      if (ent->d_type != DT_REG)
-		continue;
-
-	      size_t len = strlen (ent->d_name);
-	      const char *suffix = ".conf";
-
-	      if (len > strlen (suffix)
-		  && strcmp (ent->d_name + len - strlen (suffix), suffix) == 0)
-		{
-		  char *conf;
-		  if (__asprintf (&conf, "%s/%s", buf, ent->d_name) < 0)
-		    continue;
-		  read_conf_file (conf, elem, elem_len);
-		  free (conf);
-		}
-	    }
-	  __closedir (confdir);
-	}
-      free (buf);
-    }
+    gconv_parseconfdir (__gconv_path_elem[cnt].name,
+			__gconv_path_elem[cnt].len);
 #endif
 
   /* Add the internal modules.  */
diff --git a/iconv/gconv_parseconfdir.h b/iconv/gconv_parseconfdir.h
new file mode 100644
index 0000000000..3d4d58d4be
--- /dev/null
+++ b/iconv/gconv_parseconfdir.h
@@ -0,0 +1,161 @@ 
+/* Handle configuration data.
+   Copyright (C) 2021 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#include <dirent.h>
+#include <libc-symbols.h>
+#include <locale.h>
+#include <sys/types.h>
+
+#if IS_IN (libc)
+# include <libio/libioP.h>
+# define __getdelim(line, len, c, fp) _IO_getdelim (line, len, c, fp)
+
+# undef isspace
+# define isspace(__c) __isspace_l ((__c), _nl_C_locobj_ptr)
+# define asprintf __asprintf
+# define opendir __opendir
+# define readdir __readdir
+# define closedir __closedir
+# define mempcpy __mempcpy
+#endif
+
+/* Name of the file containing the module information in the directories
+   along the path.  */
+static const char gconv_conf_filename[] = "gconv-modules";
+static const char gconv_conf_dirname[] = "gconv-modules.d";
+
+static void add_alias (char *);
+static void add_module (char *, const char *, size_t, int);
+
+/* Read the next configuration file.  */
+static bool
+read_conf_file (const char *filename, const char *directory, size_t dir_len)
+{
+  /* Note the file is opened with cancellation in the I/O functions
+     disabled.  */
+  FILE *fp = fopen (filename, "rce");
+  char *line = NULL;
+  size_t line_len = 0;
+  static int modcounter;
+
+  /* Don't complain if a file is not present or readable, simply silently
+     ignore it.  */
+  if (fp == NULL)
+    return false;
+
+  /* No threads reading from this stream.  */
+  __fsetlocking (fp, FSETLOCKING_BYCALLER);
+
+  /* Process the known entries of the file.  Comments start with `#' and
+     end with the end of the line.  Empty lines are ignored.  */
+  while (!__feof_unlocked (fp))
+    {
+      char *rp, *endp, *word;
+      ssize_t n = __getdelim (&line, &line_len, '\n', fp);
+      if (n < 0)
+	/* An error occurred.  */
+	break;
+
+      rp = line;
+      /* Terminate the line (excluding comments or newline) by an NUL byte
+	 to simplify the following code.  */
+      endp = strchr (rp, '#');
+      if (endp != NULL)
+	*endp = '\0';
+      else
+	if (rp[n - 1] == '\n')
+	  rp[n - 1] = '\0';
+
+      while (isspace (*rp))
+	++rp;
+
+      /* If this is an empty line go on with the next one.  */
+      if (rp == endp)
+	continue;
+
+      word = rp;
+      while (*rp != '\0' && !isspace (*rp))
+	++rp;
+
+      if (rp - word == sizeof ("alias") - 1
+	  && memcmp (word, "alias", sizeof ("alias") - 1) == 0)
+	add_alias (rp);
+      else if (rp - word == sizeof ("module") - 1
+	       && memcmp (word, "module", sizeof ("module") - 1) == 0)
+	add_module (rp, directory, dir_len, modcounter++);
+      /* else */
+	/* Otherwise ignore the line.  */
+    }
+
+  free (line);
+
+  fclose (fp);
+  return true;
+}
+
+static __always_inline bool
+gconv_parseconfdir (const char *dir, size_t dir_len)
+{
+  /* No slash needs to be inserted between dir and gconv_conf_filename;
+     dir already ends in a slash.  */
+  char *buf = malloc (dir_len + sizeof (gconv_conf_dirname));
+  bool found = false;
+
+  if (buf == NULL)
+    return false;
+
+  char *cp = mempcpy (mempcpy (buf, dir, dir_len), gconv_conf_filename,
+		      sizeof (gconv_conf_filename));
+
+  /* Read the gconv-modules configuration file first.  */
+  found = read_conf_file (buf, dir, dir_len);
+
+  /* Next, see if there is a gconv-modules.d directory containing
+     configuration files and if it is non-empty.  */
+  cp--;
+  cp[0] = '.';
+  cp[1] = 'd';
+  cp[2] = '\0';
+
+  DIR *confdir = opendir (buf);
+  if (confdir != NULL)
+    {
+      struct dirent *ent;
+      while ((ent = readdir (confdir)) != NULL)
+	{
+	  if (ent->d_type != DT_REG)
+	    continue;
+
+	  size_t len = strlen (ent->d_name);
+	  const char *suffix = ".conf";
+
+	  if (len > strlen (suffix)
+	      && strcmp (ent->d_name + len - strlen (suffix), suffix) == 0)
+	    {
+	      char *conf;
+	      if (asprintf (&conf, "%s/%s", buf, ent->d_name) < 0)
+		continue;
+	      found |= read_conf_file (conf, dir, dir_len);
+	      free (conf);
+	    }
+	}
+      closedir (confdir);
+    }
+  free (buf);
+  return found;
+}