[2/5] iconvconfig: Read configuration from gconv-modules.d subdirectory

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

Commit Message

Adhemerval Zanella via Libc-alpha June 7, 2021, 8:52 a.m.
In addition to GCONV_PATH/gconv-modules, also read module
configuration from *.conf files in GCONV_PATH/gconv-modules.d.  This
allows a single gconv directory to have multiple sets of gconv modules
but at the same time, a single modules cache.

With this feature, one could separate the glibc supported gconv
modules into a minimal essential set (ISO-8859-*, UTF, etc.) from the
remaining modules.  In future, these could be further segregated into
langpack-associated sets with their own
gconv-modules.d/someconfig.conf.
---
 iconv/iconvconfig.c | 50 +++++++++++++++++++++++++++++++++++++++------
 1 file changed, 44 insertions(+), 6 deletions(-)

-- 
2.31.1

Comments

Andreas Schwab June 7, 2021, 9:06 a.m. | #1
On Jun 07 2021, Siddhesh Poyarekar via Libc-alpha wrote:

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

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

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


Should this also handle DT_UNKNOWN?

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."
Adhemerval Zanella via Libc-alpha June 7, 2021, 9:17 a.m. | #2
On 6/7/21 2:36 PM, Andreas Schwab wrote:
> On Jun 07 2021, Siddhesh Poyarekar via Libc-alpha wrote:

> 

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

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

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

> 

> Should this also handle DT_UNKNOWN?


I suppose I could fall back to lstat to get the file type if dt_type is 
not supported by the underlying filesystem.

Siddhesh
Adhemerval Zanella via Libc-alpha June 8, 2021, 9:46 p.m. | #3
Siddhesh Poyarekar via Libc-alpha <libc-alpha@sourceware.org> writes:
> In addition to GCONV_PATH/gconv-modules, also read module

> configuration from *.conf files in GCONV_PATH/gconv-modules.d.  This

> allows a single gconv directory to have multiple sets of gconv modules

> but at the same time, a single modules cache.

>

> With this feature, one could separate the glibc supported gconv

> modules into a minimal essential set (ISO-8859-*, UTF, etc.) from the

> remaining modules.  In future, these could be further segregated into

> langpack-associated sets with their own

> gconv-modules.d/someconfig.conf.

> ---

>  iconv/iconvconfig.c | 50 +++++++++++++++++++++++++++++++++++++++------

>  1 file changed, 44 insertions(+), 6 deletions(-)


> diff --git a/iconv/iconvconfig.c b/iconv/iconvconfig.c

> +#include <dirent.h>


Ok.

> +#include <sys/types.h>


Ok.

> @@ -710,6 +712,7 @@ handle_file (const char *dir, const char *infile)

>  static int

>  handle_dir (const char *dir)

>  {

> +#define BUF_LEN prefix_len + dirlen + sizeof "gconv-modules.d"


sizeof includes one for the NUL, so OK.  

> @@ -722,20 +725,55 @@ handle_dir (const char *dir)

>        newp[dirlen] = '\0';

>      }

>  

> -  char infile[prefix_len + dirlen + sizeof "gconv-modules"];

> -  cp = infile;

> +  /* First, look for a gconv-modules file.  */

> +  char buf[BUF_LEN];

> +  cp = buf;

>    if (dir[0] == '/')

>      cp = mempcpy (cp, prefix, prefix_len);

> -  strcpy (mempcpy (cp, dir, dirlen), "gconv-modules");

> +  cp = mempcpy (cp, dir, dirlen);

> +  cp = stpcpy (cp, "gconv-modules");


max string is "<prefix><dir>gconv-modules"

> -  found |= handle_file (dir, infile);

> +  found |= handle_file (dir, buf);


Looks for gconv-modules files, ok.

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

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

> +  cp[0] = '.';

> +  cp[1] = 'd';

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


buf[] is now "...gconv-modules.d"

> +  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)

> +	    {

> +	      /* LEN <= PATH_MAX so this alloca is not unbounded.  */


But it doesn't go out of scope until the function *exits* although one
could assume that the compiler would re-use (overlap) them.  Shouldn't
be a problem unless there's a stupid number of conf files in a subdir.

> +	      char *conf = alloca (BUF_LEN + len + 1);


The byte for NUL in BUF_LEN is reused here, the "1" is for "/".  Ok.

> +	      cp = stpcpy (conf, buf);

> +	      sprintf (cp, "/%s", ent->d_name);

> +	      found |= handle_file (dir, conf);

> +	    }

> +	}

> +      closedir (confdir);

> +    }


Ok.

>    if (!found)

>      {

> -      error (0, errno, "failed to open gconv configuration file in `%s'",

> +      error (0, errno, "failed to open gconv configuration files in `%s'",

>  	     dir);

>        error (0, 0,

> -	     "ensure that the directory contains a valid gconv-modules file.");

> +	     "ensure that the directory contains either a valid "

> +	     "gconv-modules file or a gconv-modules.d directory with "

> +	     "configuration files with names ending in .conf.");

>      }


Ok.

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

Patch

diff --git a/iconv/iconvconfig.c b/iconv/iconvconfig.c
index f5aba636d7..b2a868919c 100644
--- a/iconv/iconvconfig.c
+++ b/iconv/iconvconfig.c
@@ -18,6 +18,7 @@ 
 
 #include <argp.h>
 #include <assert.h>
+#include <dirent.h>
 #include <error.h>
 #include <errno.h>
 #include <fcntl.h>
@@ -33,6 +34,7 @@ 
 #include <string.h>
 #include <unistd.h>
 #include <sys/cdefs.h>
+#include <sys/types.h>
 #include <sys/uio.h>
 
 #include "iconvconfig.h"
@@ -710,6 +712,7 @@  handle_file (const char *dir, const char *infile)
 static int
 handle_dir (const char *dir)
 {
+#define BUF_LEN prefix_len + dirlen + sizeof "gconv-modules.d"
   char *cp;
   size_t dirlen = strlen (dir);
   bool found = false;
@@ -722,20 +725,55 @@  handle_dir (const char *dir)
       newp[dirlen] = '\0';
     }
 
-  char infile[prefix_len + dirlen + sizeof "gconv-modules"];
-  cp = infile;
+  /* First, look for a gconv-modules file.  */
+  char buf[BUF_LEN];
+  cp = buf;
   if (dir[0] == '/')
     cp = mempcpy (cp, prefix, prefix_len);
-  strcpy (mempcpy (cp, dir, dirlen), "gconv-modules");
+  cp = mempcpy (cp, dir, dirlen);
+  cp = stpcpy (cp, "gconv-modules");
 
-  found |= handle_file (dir, infile);
+  found |= handle_file (dir, buf);
+
+  /* Next, see if there is a gconv-modules.d directory containing configuration
+     files and if it is non-empty.  */
+  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)
+	    {
+	      /* LEN <= PATH_MAX so this alloca is not unbounded.  */
+	      char *conf = alloca (BUF_LEN + len + 1);
+	      cp = stpcpy (conf, buf);
+	      sprintf (cp, "/%s", ent->d_name);
+	      found |= handle_file (dir, conf);
+	    }
+	}
+      closedir (confdir);
+    }
 
   if (!found)
     {
-      error (0, errno, "failed to open gconv configuration file in `%s'",
+      error (0, errno, "failed to open gconv configuration files in `%s'",
 	     dir);
       error (0, 0,
-	     "ensure that the directory contains a valid gconv-modules file.");
+	     "ensure that the directory contains either a valid "
+	     "gconv-modules file or a gconv-modules.d directory with "
+	     "configuration files with names ending in .conf.");
     }
 
   return found ? 0 : 1;