From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qk1-x731.google.com (mail-qk1-x731.google.com [IPv6:2607:f8b0:4864:20::731]) by sourceware.org (Postfix) with ESMTPS id 419A839A2402 for ; Wed, 9 Jun 2021 17:21:32 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 419A839A2402 Received: by mail-qk1-x731.google.com with SMTP id j184so24430532qkd.6 for ; Wed, 09 Jun 2021 10:21:32 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=CaiGPB8rTWY8naxdg0XvZHYCREUHF50hmCYoGLIbv/o=; b=Wm2twUzyIbwfXRbLSilWRfXsrX0jw/1NVJT0dDFhT+4p8MaAmwbF2b59664REONM+d /63KjEIoK+n/uCxma9r9Jli5/USvjVxj0mVrsvlScPGw9o2kuNPyr4N4IHsAfDsV/y7P Md3wjwlAz6jwSqbDuk24deGJ61253/V1Yniv4JGHSMNOdC1BOwBwWe9Im0hRCJsc4jAx tIGUa4Ruvjx+vyixA0Z6ADny6BIwg9Oi7cRsXlvLF1OpYtITn32yD2LXk6Av7c2wamGn t6k5fJv7yNALX3FvIpQzImdvLL2med2L8A9sfq+fvA163XGPCcy9LqWiGWefZRnbW029 xQIQ== X-Gm-Message-State: AOAM532ls0WapXrwuk8xs8xERS5EBPpx7DRUddPJSjaeo5bfVnX3q8it 3rhFM9/6SL2Tf4r62M3aPgoL/g== X-Google-Smtp-Source: ABdhPJz5GjK0c95A5eGlgyCY6KAisOzTynhuU0IVkMSDVSRCGMX0SBL4+/f/c1xwXHpA1L4BS/7nZQ== X-Received: by 2002:a05:620a:6b3:: with SMTP id i19mr675243qkh.55.1623259291736; Wed, 09 Jun 2021 10:21:31 -0700 (PDT) Received: from [192.168.1.4] ([177.194.59.218]) by smtp.gmail.com with ESMTPSA id p21sm391556qtq.92.2021.06.09.10.21.30 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 09 Jun 2021 10:21:31 -0700 (PDT) Subject: Re: [PATCH] Handle DT_UNKNOWN in gconv-modules.d To: Siddhesh Poyarekar , libc-alpha@sourceware.org Cc: schwab@linux-m68k.org References: <20210609043835.218509-1-siddhesh@sourceware.org> From: Adhemerval Zanella Message-ID: Date: Wed, 9 Jun 2021 14:21:28 -0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.8.1 MIME-Version: 1.0 In-Reply-To: <20210609043835.218509-1-siddhesh@sourceware.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-12.4 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, 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: Wed, 09 Jun 2021 17:21:33 -0000 On 09/06/2021 01:38, Siddhesh Poyarekar via Libc-alpha wrote: > On filesystems that do not support dt_type, a regular file shows up as > DT_UNKNOWN. Fall back to using lstat64 to read file properties in > such cases. The patch looks ok, but two things raised checking on this code: 1. the code is essentially the same on both places and 2. the use of alloca() even when it is assured that is bounded and 2. The former would be nice if could consolidate it (even by adding a file where both iconvconfig and iconv could include or even by a GLIBC_PRIVATE symbol), but it is not a deal breaker. But I think we should move away from alloca, even when we know it is bounded (sorry if I didn't catch on the previous patch). For this specific usage we can use asprintf to create the path or use a static PATH_MAX buffer. > --- > iconv/gconv_conf.c | 9 ++++++++- > iconv/iconvconfig.c | 8 +++++++- > 2 files changed, 15 insertions(+), 2 deletions(-) > > diff --git a/iconv/gconv_conf.c b/iconv/gconv_conf.c > index c8ad8099a4..7fc3a810af 100644 > --- a/iconv/gconv_conf.c > +++ b/iconv/gconv_conf.c > @@ -587,7 +587,7 @@ __gconv_read_conf (void) > struct dirent *ent; > while ((ent = __readdir (confdir)) != NULL) > { > - if (ent->d_type != DT_REG) > + if (ent->d_type != DT_REG && ent->d_type != DT_UNKNOWN) > continue; > > size_t len = strlen (ent->d_name); > @@ -596,10 +596,17 @@ __gconv_read_conf (void) > if (len > strlen (suffix) > && strcmp (ent->d_name + len - strlen (suffix), suffix) == 0) > { > + struct stat64 st; > /* LEN <= PATH_MAX so this alloca is not unbounded. */ > char *conf = alloca (BUF_LEN + len + 1); > cp = stpcpy (conf, buf); > sprintf (cp, "/%s", ent->d_name); > + > + if (ent->d_type == DT_UNKNOWN > + && (__lstat64 (conf, &st) == -1 > + || !S_ISREG (st.st_mode))) > + continue; > + > read_conf_file (conf, elem, elem_len, &modules, &nmodules); > } > } > diff --git a/iconv/iconvconfig.c b/iconv/iconvconfig.c > index b2a868919c..8f10f4aba8 100644 > --- a/iconv/iconvconfig.c > +++ b/iconv/iconvconfig.c > @@ -747,7 +747,7 @@ handle_dir (const char *dir) > struct dirent *ent; > while ((ent = readdir (confdir)) != NULL) > { > - if (ent->d_type != DT_REG) > + if (ent->d_type != DT_REG && ent->d_type != DT_UNKNOWN) > continue; > > size_t len = strlen (ent->d_name); > @@ -756,10 +756,16 @@ handle_dir (const char *dir) > if (len > strlen (suffix) > && strcmp (ent->d_name + len - strlen (suffix), suffix) == 0) > { > + struct stat64 st; > /* LEN <= PATH_MAX so this alloca is not unbounded. */ > char *conf = alloca (BUF_LEN + len + 1); > cp = stpcpy (conf, buf); > sprintf (cp, "/%s", ent->d_name); > + > + if (ent->d_type == DT_UNKNOWN > + && (lstat64 (conf, &st) == -1 || !S_ISREG (st.st_mode))) > + continue; > + > found |= handle_file (dir, conf); > } > } >