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

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

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.
  

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
  

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>