From: Szabolcs Nagy <szabolcs.nagy@arm.com>
To: Siddhesh Poyarekar <siddhesh@sourceware.org>
Cc: libc-alpha@sourceware.org, schwab@linux-m68k.org
Subject: Re: [PATCH 4/6] iconvconfig: Use common gconv module parsing function
Date: Fri, 2 Jul 2021 10:11:11 +0100 [thread overview]
Message-ID: <20210702091109.GG14854@arm.com> (raw)
In-Reply-To: <20210610111853.2286873-5-siddhesh@sourceware.org>
The 06/10/2021 16:48, Siddhesh Poyarekar via Libc-alpha wrote:
> 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.
build fails for me after this with
iconvconfig.c:(.text+0x7ec): undefined reference to `__feof_unlocked'
if i configure with -Os --disable-werror
i.e. the patch relies on __USE_EXTERN_INLINES to inline __feof_unlocked
(which is not a public symbol so should not be referenced from iconv binaries)
> ---
> iconv/iconvconfig.c | 126 ++++----------------------------------------
> 1 file changed, 11 insertions(+), 115 deletions(-)
>
> diff --git a/iconv/iconvconfig.c b/iconv/iconvconfig.c
> index c9607fb645..53e5c5c6f4 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>
>
> /* Get libc version number. */
> #include "../version.h"
> @@ -568,7 +567,7 @@ 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, int modcount)
> {
> /* We expect now
> 1. `from' name
> @@ -646,131 +645,28 @@ add_module (char *rp, const char *directory)
> cost, need_ext);
> }
>
> -/* 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;
> -}
> -
> /* 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);
> +
> if (dir[dirlen - 1] != '/')
> {
> - char *newp = (char *) xmalloc (dirlen + 2);
> - dir = memcpy (newp, dir, dirlen);
> - newp[dirlen++] = '/';
> - newp[dirlen] = '\0';
> + *cp++ = '/';
> + *cp = '\0';
> + dirlen += 2;
> }
>
> - /* 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);
>
> - free (buf);
> + free (fulldir);
>
> -out:
> if (!found)
> {
> error (0, errno, "failed to open gconv configuration files in `%s'",
> --
> 2.31.1
>
--
next prev parent reply other threads:[~2021-07-02 9:11 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-10 11:18 [PATCH 0/6] gconv configuration parsing cleanups Siddhesh Poyarekar
2021-06-10 11:18 ` [PATCH 1/6] iconv: Remove alloca use in gconv-modules configuration parsing Siddhesh Poyarekar
2021-06-22 3:17 ` DJ Delorie
2021-06-10 11:18 ` [PATCH 2/6] gconv_conf: Remove unused variables Siddhesh Poyarekar
2021-06-16 15:28 ` Andreas Schwab
2021-06-22 3:23 ` DJ Delorie
2021-06-10 11:18 ` [PATCH 3/6] gconv_conf: Split out configuration file processing Siddhesh Poyarekar
2021-06-22 3:48 ` DJ Delorie
2021-06-10 11:18 ` [PATCH 4/6] iconvconfig: Use common gconv module parsing function Siddhesh Poyarekar
2021-06-22 4:01 ` DJ Delorie
2021-07-02 9:11 ` Szabolcs Nagy [this message]
2021-07-02 9:27 ` Szabolcs Nagy
2021-06-10 11:18 ` [PATCH 5/6] Handle DT_UNKNOWN in gconv-modules.d Siddhesh Poyarekar
2021-06-22 4:03 ` DJ Delorie
2021-06-10 11:18 ` [PATCH 6/6] Add NEWS item for gconv-modules.d change Siddhesh Poyarekar
2021-06-10 11:37 ` Siddhesh Poyarekar
2021-06-16 4:02 ` [PING][PATCH 0/6] gconv configuration parsing cleanups Siddhesh Poyarekar
2021-06-21 9:14 ` [Ping 2][PATCH " 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=20210702091109.GG14854@arm.com \
--to=szabolcs.nagy@arm.com \
--cc=libc-alpha@sourceware.org \
--cc=schwab@linux-m68k.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).