[4/6] iconvconfig: Use common gconv module parsing function
Checks
Commit Message
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 | 126 ++++----------------------------------------
1 file changed, 11 insertions(+), 115 deletions(-)
Comments
Siddhesh Poyarekar <siddhesh@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 | 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>
Ok.
> /* Add new module. */
> static void
> -add_module (char *rp, const char *directory)
> +add_module (char *rp, const char *directory, size_t dirlen, int modcount)
Do we need some sort of attribute_unused here?
Otherwise OK.
> @@ -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;
> -}
> -
No longer needed; 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);
> +
+1 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 += 2;
> }
Why +=2 and not ++ ? We don't want to be mempcpy'ing the NUL into the
target string in the .h.
> - /* 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);
Ok.
> -out:
> if (!found)
> {
> error (0, errno, "failed to open gconv configuration files in `%s'",
Ok.
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
>
--
The 07/02/2021 10:11, Szabolcs Nagy via Libc-alpha wrote:
> if i configure with -Os --disable-werror
i meant CLFAGS=-Os --disable-werror
@@ -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'",