From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by sourceware.org (Postfix) with ESMTP id 7589A3848010 for ; Tue, 22 Jun 2021 03:48:24 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 7589A3848010 Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-561-TZxwKAx1NWyRbtL9XA5JYQ-1; Mon, 21 Jun 2021 23:48:21 -0400 X-MC-Unique: TZxwKAx1NWyRbtL9XA5JYQ-1 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 984BC81901D; Tue, 22 Jun 2021 03:48:20 +0000 (UTC) Received: from greed.delorie.com (ovpn-112-111.rdu2.redhat.com [10.10.112.111]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 5989760C13; Tue, 22 Jun 2021 03:48:20 +0000 (UTC) Received: from greed.delorie.com.redhat.com (localhost [127.0.0.1]) by greed.delorie.com (8.15.2/8.15.2) with ESMTP id 15M3mIXe408562; Mon, 21 Jun 2021 23:48:18 -0400 From: DJ Delorie To: Siddhesh Poyarekar Cc: libc-alpha@sourceware.org, adhemerval.zanella@linaro.org, schwab@linux-m68k.org Subject: Re: [PATCH 3/6] gconv_conf: Split out configuration file processing In-Reply-To: <20210610111853.2286873-4-siddhesh@sourceware.org> Date: Mon, 21 Jun 2021 23:48:18 -0400 Message-ID: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain X-Spam-Status: No, score=-12.6 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 22 Jun 2021 03:48:26 -0000 Siddhesh Poyarekar writes: > Split configuration file processing into a separate header file and > include it. Macroize all calls that need to go through internal > interfaces so that iconvconfig can also use them. > -#include > #include > #include > #include > @@ -31,11 +30,10 @@ > #include > #include > #include > -#include > > #include > #include > - > +#include Ok; gconv_parseconfdir.h includes those two deleted ones. > -/* Name of the file containing the module information in the directories > - along the path. */ > -static const char gconv_conf_filename[] = "gconv-modules"; > -static const char gconv_conf_dirname[] = "gconv-modules.d"; > - Moved to .h; ok. > -#include > -#define __getdelim(line, len, c, fp) _IO_getdelim (line, len, c, fp) > - Moved, ok. > -/* Read the next configuration file. */ > -static void > -read_conf_file (const char *filename, const char *directory, size_t dir_len) > -{ > - /* Note the file is opened with cancellation in the I/O functions > - disabled. */ > - FILE *fp = fopen (filename, "rce"); > - char *line = NULL; > - size_t line_len = 0; > - static int modcounter; > - > - /* Don't complain if a file is not present or readable, simply silently > - ignore it. */ > - if (fp == NULL) > - return; > - > - /* No threads reading from this stream. */ > - __fsetlocking (fp, FSETLOCKING_BYCALLER); > - > - /* Process the known entries of the file. Comments start with `#' and > - end with the end of the line. Empty lines are ignored. */ > - while (!__feof_unlocked (fp)) > - { > - char *rp, *endp, *word; > - ssize_t n = __getdelim (&line, &line_len, '\n', fp); > - if (n < 0) > - /* An error occurred. */ > - break; > - > - rp = line; > - /* Terminate the line (excluding comments or newline) by an 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_l (*rp, _nl_C_locobj_ptr)) > - ++rp; > - > - /* If this is an empty line go on with the next one. */ > - if (rp == endp) > - continue; > - > - word = rp; > - while (*rp != '\0' && !__isspace_l (*rp, _nl_C_locobj_ptr)) > - ++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, directory, dir_len, modcounter++); > - /* else */ > - /* Otherwise ignore the line. */ > - } > - > - free (line); > - > - fclose (fp); > -} > - > - Moved to .h; ok. > @@ -554,55 +478,8 @@ __gconv_read_conf (void) > __gconv_get_path (); > > for (cnt = 0; __gconv_path_elem[cnt].name != NULL; ++cnt) > - { > - const char *elem = __gconv_path_elem[cnt].name; > - size_t elem_len = __gconv_path_elem[cnt].len; > - > - /* No slash needs to be inserted between elem and gconv_conf_filename; > - elem already ends in a slash. */ > - char *buf = malloc (elem_len + sizeof (gconv_conf_dirname)); > - if (buf == NULL) > - continue; > - > - char *cp = __mempcpy (__mempcpy (buf, elem, elem_len), > - gconv_conf_filename, sizeof (gconv_conf_filename)); > - > - /* Read the gconv-modules configuration file first. */ > - read_conf_file (buf, elem, elem_len); > - > - /* Next, see if there is a gconv-modules.d directory containing > - configuration files and if it is non-empty. */ > - cp--; > - 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; > - read_conf_file (conf, elem, elem_len); > - free (conf); > - } > - } > - __closedir (confdir); > - } > - free (buf); > - } > + gconv_parseconfdir (__gconv_path_elem[cnt].name, > + __gconv_path_elem[cnt].len); > #endif Ok, found in .h. > diff --git a/iconv/gconv_parseconfdir.h b/iconv/gconv_parseconfdir.h > new file mode 100644 > index 0000000000..3d4d58d4be > --- /dev/null > +++ b/iconv/gconv_parseconfdir.h > @@ -0,0 +1,161 @@ > +/* Handle configuration data. > + Copyright (C) 2021 Free Software Foundation, Inc. > + This file is part of the GNU C Library. > + > + The GNU C Library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Lesser General Public > + License as published by the Free Software Foundation; either > + version 2.1 of the License, or (at your option) any later version. > + > + The GNU C Library is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + Lesser General Public License for more details. > + > + You should have received a copy of the GNU Lesser General Public > + License along with the GNU C Library; if not, see > + . */ Ok. > +#include > +#include > +#include > +#include Ok. > +#if IS_IN (libc) > +# include > +# define __getdelim(line, len, c, fp) _IO_getdelim (line, len, c, fp) > + > +# undef isspace > +# define isspace(__c) __isspace_l ((__c), _nl_C_locobj_ptr) > +# define asprintf __asprintf > +# define opendir __opendir > +# define readdir __readdir > +# define closedir __closedir > +# define mempcpy __mempcpy > +#endif Ok. > +/* Name of the file containing the module information in the directories > + along the path. */ > +static const char gconv_conf_filename[] = "gconv-modules"; > +static const char gconv_conf_dirname[] = "gconv-modules.d"; Ok. > +static void add_alias (char *); > +static void add_module (char *, const char *, size_t, int); Ok. > +/* Read the next configuration file. */ > +static bool > +read_conf_file (const char *filename, const char *directory, size_t dir_len) > +{ > + /* Note the file is opened with cancellation in the I/O functions > + disabled. */ > + FILE *fp = fopen (filename, "rce"); > + char *line = NULL; > + size_t line_len = 0; > + static int modcounter; > + > + /* Don't complain if a file is not present or readable, simply silently > + ignore it. */ > + if (fp == NULL) > + return false; > + > + /* No threads reading from this stream. */ > + __fsetlocking (fp, FSETLOCKING_BYCALLER); > + > + /* Process the known entries of the file. Comments start with `#' and > + end with the end of the line. Empty lines are ignored. */ > + while (!__feof_unlocked (fp)) > + { > + char *rp, *endp, *word; > + ssize_t n = __getdelim (&line, &line_len, '\n', fp); > + if (n < 0) > + /* An error occurred. */ > + break; > + > + rp = line; > + /* Terminate the line (excluding comments or newline) by an 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, directory, dir_len, modcounter++); > + /* else */ > + /* Otherwise ignore the line. */ > + } > + > + free (line); > + > + fclose (fp); > + return true; > +} > + Copied from above; ok. > +static __always_inline bool > +gconv_parseconfdir (const char *dir, size_t dir_len) > +{ > + /* No slash needs to be inserted between dir and gconv_conf_filename; > + dir already ends in a slash. */ > + char *buf = malloc (dir_len + sizeof (gconv_conf_dirname)); > + bool found = false; > + > + if (buf == NULL) > + return false; > + > + char *cp = mempcpy (mempcpy (buf, dir, dir_len), gconv_conf_filename, > + sizeof (gconv_conf_filename)); > + > + /* Read the gconv-modules configuration file first. */ > + found = read_conf_file (buf, dir, dir_len); > + > + /* Next, see if there is a gconv-modules.d directory containing > + configuration files and if it is non-empty. */ > + cp--; > + 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 |= read_conf_file (conf, dir, dir_len); > + free (conf); > + } > + } > + closedir (confdir); > + } > + free (buf); > + return found; > +} Extracted from above; ok. LGTM. Reviewed-by: DJ Delorie