From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.cs.ucla.edu (mail.cs.ucla.edu [131.179.128.66]) by sourceware.org (Postfix) with ESMTPS id 4E57E3858CD1 for ; Fri, 14 Jul 2023 16:11:09 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 4E57E3858CD1 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=cs.ucla.edu Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=cs.ucla.edu Received: from localhost (localhost [127.0.0.1]) by mail.cs.ucla.edu (Postfix) with ESMTP id 163833C011BD4; Fri, 14 Jul 2023 09:11:08 -0700 (PDT) Received: from mail.cs.ucla.edu ([127.0.0.1]) by localhost (mail.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10032) with ESMTP id OpEZxtBwcvbv; Fri, 14 Jul 2023 09:11:07 -0700 (PDT) Received: from localhost (localhost [127.0.0.1]) by mail.cs.ucla.edu (Postfix) with ESMTP id C2B283C011BDC; Fri, 14 Jul 2023 09:11:07 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.10.3 mail.cs.ucla.edu C2B283C011BDC DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cs.ucla.edu; s=9D0B346E-2AEB-11ED-9476-E14B719DCE6C; t=1689351067; bh=M85KpwGkI8eA9PXhT64k2liqZU8wfWyAxPWlIDgVu+k=; h=Message-ID:Date:MIME-Version:To:From; b=JSXcWLswyK/vQ7VWVSr96pb/vKldgqMPjXxRWlqPU3zHar0Sh2AOljk54MrLkDU9Z SnzaUZ65sndgNAOq1w3hl1rahNHEb0FeCDRYHqQDEy9BHwCB6f8f62QWP15K756W2p VgQs9Et2QhxuMAhEMfVHlk/awSLy7hoLLR0Mg0hAkI1OAPYnx6lgDBDu+BVwTo4o65 PyzrIbLcupaH/XVQk+iN1YPtb0s2Z6JVGvNApUd9AB6S1tEFpQcmhHV+NM1fIO4Qys rOirrp7g1oM3OB7t/8eugJUM13grLUZZjMzegwPYA79tbuL0mR388q9lxq35FKVG4j GqiCQ2MhRjDGA== X-Virus-Scanned: amavisd-new at mail.cs.ucla.edu Received: from mail.cs.ucla.edu ([127.0.0.1]) by localhost (mail.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10026) with ESMTP id PfhqVBAXAnD0; Fri, 14 Jul 2023 09:11:07 -0700 (PDT) Received: from [192.168.1.9] (cpe-172-91-119-151.socal.res.rr.com [172.91.119.151]) by mail.cs.ucla.edu (Postfix) with ESMTPSA id 9EA0A3C011BD4; Fri, 14 Jul 2023 09:11:07 -0700 (PDT) Message-ID: Date: Fri, 14 Jul 2023 09:11:07 -0700 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.13.0 To: Stanley Lancaster References: <20230714145224.1456633-1-lancasterharp@gmail.com> Content-Language: en-US From: Paul Eggert Organization: UCLA Computer Science Department Cc: libc-alpha@sourceware.org Subject: Re: Fix %Z parsing in strptime [BZ #16088] In-Reply-To: <20230714145224.1456633-1-lancasterharp@gmail.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-3.0 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On 2023-07-14 07:52, Stanley Lancaster via Libc-alpha wrote: > /* Read timezone but perform no conversion. */ > + /* we recognize the format [-+a-zA-Z0-9]{3,} */ Use GNU style in comment with active voice sentences and two spaces after sentence end, e.g., "/* Read time zone but perform no conversion. Recognize the format [-+a-zA-Z0-9]{3,}. */". > + const char* stop_rp = rp + 3; Again, GNU style: "char *stop_rp" not "char* stop_rp". More important, this has undefined behavior if rp + 3 overflows. Instead, count the number of bytes after the loop finishes, and make sure it's 3 or more. > + while (((*rp >= 'A' && *rp <= 'Z') || (*rp >= 'a' && *rp <= 'z') || (*rp >= '0' && *rp <= '9')) && (rp < stop_rp) && *rp != '\0') Omit "&& *rp != '\0'"; it's redundant. Reindent to 80 columns. > + { "C", "1999CST0502123412", "%Y%Z%m%d%H%M%S", 0, 121, 4, 2 }, I don't see how this test passes. "CST0502123412" is treated as a time zone abbreviation, so the only info is the year. Did you run the tests? If so, why did this test pass? If not, please run. Since the patch does not fix BZ#16088, it needs a commit message that describes what the patch does and why it's a win even though it doesn't fix. In particular, the patch does not set tm_zone, and there's a reason for that, and this should be explained.