From: DJ Delorie <dj@redhat.com>
To: Siddhesh Poyarekar <siddhesh@sourceware.org>
Cc: libc-alpha@sourceware.org
Subject: Re: [PATCH v2 4/6] iconvconfig: Use common gconv module parsing function
Date: Tue, 22 Jun 2021 01:27:32 -0400 [thread overview]
Message-ID: <xnv96679kr.fsf@greed.delorie.com> (raw)
In-Reply-To: <20210622051202.4155709-5-siddhesh@sourceware.org>
Siddhesh Poyarekar via Libc-alpha <libc-alpha@sourceware.org> writes:
> Drop local copy of gconv file parsing and use the one in
> gconv_parseconfdir.h instead. Now there is a single implementation of
> configuration file parsing.
> ---
> iconv/iconvconfig.c | 128 +++++---------------------------------------
> 1 file changed, 13 insertions(+), 115 deletions(-)
>
> diff --git a/iconv/iconvconfig.c b/iconv/iconvconfig.c
> index c9607fb645..e69334d71c 100644
> --- a/iconv/iconvconfig.c
> +++ b/iconv/iconvconfig.c
> @@ -18,7 +18,6 @@
>
> #include <argp.h>
> #include <assert.h>
> -#include <dirent.h>
> #include <error.h>
> #include <errno.h>
> #include <fcntl.h>
> @@ -34,10 +33,10 @@
> #include <string.h>
> #include <unistd.h>
> #include <sys/cdefs.h>
> -#include <sys/types.h>
> #include <sys/uio.h>
>
> #include "iconvconfig.h"
> +#include <gconv_parseconfdir.h>
Ok.
> /* Get libc version number. */
> #include "../version.h"
> @@ -568,7 +567,9 @@ new_module (const char *fromname, size_t fromlen, const char *toname,
>
> /* Add new module. */
> static void
> -add_module (char *rp, const char *directory)
> +add_module (char *rp, const char *directory,
> + size_t dirlen __attribute__ ((__unused__)),
> + int modcount __attribute__ ((__unused__)))
Ok.
> -/* Read a gconv-modules configuration file. */
> -static bool
> -handle_file (const char *dir, const char *infile)
> -{
> - FILE *fp;
> - char *line = NULL;
> - size_t linelen = 0;
> -
> - fp = fopen (infile, "r");
> - if (fp == NULL)
> - return false;
> -
> - /* No threads present. */
> - __fsetlocking (fp, FSETLOCKING_BYCALLER);
> -
> - while (!feof_unlocked (fp))
> - {
> - char *rp, *endp, *word;
> - ssize_t n = __getdelim (&line, &linelen, '\n', fp);
> -
> - if (n < 0)
> - /* An error occurred. */
> - break;
> -
> - rp = line;
> - /* Terminate the line (excluding comments or newline) with a 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, dir);
> - /* else */
> - /* Otherwise ignore the line. */
> - }
> -
> - free (line);
> -
> - fclose (fp);
> -
> - return true;
> -}
> -
Ok.
> /* 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;
>
> + /* Add the prefix before sending it off to the parser. */
> + char *fulldir = xmalloc (prefix_len + dirlen + 2);
> + char *cp = mempcpy (mempcpy (fulldir, prefix, prefix_len), dir, dirlen);
> +
+2 for '/' and NUL, ok.
> if (dir[dirlen - 1] != '/')
> {
> - char *newp = (char *) xmalloc (dirlen + 2);
> - dir = memcpy (newp, dir, dirlen);
> - newp[dirlen++] = '/';
> - newp[dirlen] = '\0';
> + *cp++ = '/';
> + *cp = '\0';
> + dirlen++;
> }
Ok.
> - /* First, look for a gconv-modules file. */
> - char *buf = malloc (prefix_len + dirlen + sizeof "gconv-modules.d");
> - if (buf == NULL)
> - goto out;
> -
> - cp = buf;
> - if (dir[0] == '/')
> - cp = mempcpy (cp, prefix, prefix_len);
> - cp = mempcpy (cp, dir, dirlen);
> - cp = stpcpy (cp, "gconv-modules");
> -
> - 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)
> - {
> - char *conf;
> - if (asprintf (&conf, "%s/%s", buf, ent->d_name) < 0)
> - continue;
> - found |= handle_file (dir, conf);
> - free (conf);
> - }
> - }
> - closedir (confdir);
> - }
> + found = gconv_parseconfdir (fulldir, dirlen + prefix_len);
Ok.
> - free (buf);
> + free (fulldir);
>
> -out:
> if (!found)
> {
> error (0, errno, "failed to open gconv configuration files in `%s'",
Ok.
Reviewed-by: DJ Delorie <dj@redhat.com>
next prev parent reply other threads:[~2021-06-22 5:27 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-22 5:11 [PATCH v2 0/6] gconv configuration parsing cleanups Siddhesh Poyarekar
2021-06-22 5:11 ` [PATCH v2 1/6] iconv: Remove alloca use in gconv-modules configuration parsing Siddhesh Poyarekar
2021-06-22 5:11 ` [PATCH v2 2/6] gconv_conf: Remove unused variables Siddhesh Poyarekar
2021-06-22 5:11 ` [PATCH v2 3/6] gconv_conf: Split out configuration file processing Siddhesh Poyarekar
2021-06-22 5:12 ` [PATCH v2 4/6] iconvconfig: Use common gconv module parsing function Siddhesh Poyarekar
2021-06-22 5:27 ` DJ Delorie [this message]
2021-06-22 5:12 ` [PATCH v2 5/6] Handle DT_UNKNOWN in gconv-modules.d Siddhesh Poyarekar
2021-06-22 5:12 ` [PATCH v2 6/6] Add NEWS item for gconv-modules.d change Siddhesh Poyarekar
2021-06-23 3:10 ` DJ Delorie
2021-06-24 8:46 ` [PATCH v2 0/6] gconv configuration parsing cleanups Andreas Schwab
2021-06-24 9:18 ` Siddhesh Poyarekar
2021-06-24 9:23 ` Andreas Schwab
2021-06-24 15:33 ` Siddhesh Poyarekar
2021-06-24 15:46 ` Andreas Schwab
2021-06-24 15:57 ` Siddhesh Poyarekar
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xnv96679kr.fsf@greed.delorie.com \
--to=dj@redhat.com \
--cc=libc-alpha@sourceware.org \
--cc=siddhesh@sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).