From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-Status: No, score=-4.9 required=3.0 tests=AWL,BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,MAILING_LIST_MULTI,NICE_REPLY_A, SPF_HELO_PASS,SPF_PASS shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by dcvr.yhbt.net (Postfix) with ESMTPS id 702F11F990 for ; Thu, 6 Aug 2020 14:46:33 +0000 (UTC) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 95F4C384B806; Thu, 6 Aug 2020 14:46:32 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 95F4C384B806 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1596725192; bh=yn2fSDfpeCSOx3wX3r0FPtT9F/xjuetkdwFhH+rZQT4=; h=Subject:To:References:Date:In-Reply-To:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=dW3WJIOK6rTVkiSAyMqx79Yb3uj7kMAWvqzKJmrIogVHVKGCeNXuaNcQ5gXITJeB7 ibhW4AaCTDAf3HJAqw+l/mopfGhl5qRPyKws4xoT9SA40qVMKlLegIB6ur3ZOlqoX+ 8N0Ipgjp6jnqc1tWt7EScER81z57XIBFPn+mk9Rs= Received: from mx0a-001b2d01.pphosted.com (mx0b-001b2d01.pphosted.com [148.163.158.5]) by sourceware.org (Postfix) with ESMTPS id 5B2803861030 for ; Thu, 6 Aug 2020 14:46:29 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 5B2803861030 Received: from pps.filterd (m0098414.ppops.net [127.0.0.1]) by mx0b-001b2d01.pphosted.com (8.16.0.42/8.16.0.42) with SMTP id 076EVgNI141535 for ; Thu, 6 Aug 2020 10:46:29 -0400 Received: from ppma03wdc.us.ibm.com (ba.79.3fa9.ip4.static.sl-reverse.com [169.63.121.186]) by mx0b-001b2d01.pphosted.com with ESMTP id 32rjd9kcgm-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Thu, 06 Aug 2020 10:46:28 -0400 Received: from pps.filterd (ppma03wdc.us.ibm.com [127.0.0.1]) by ppma03wdc.us.ibm.com (8.16.0.42/8.16.0.42) with SMTP id 076Ejo6o018769 for ; Thu, 6 Aug 2020 14:46:28 GMT Received: from b01cxnp23032.gho.pok.ibm.com (b01cxnp23032.gho.pok.ibm.com [9.57.198.27]) by ppma03wdc.us.ibm.com with ESMTP id 32n019j7hj-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Thu, 06 Aug 2020 14:46:28 +0000 Received: from b01ledav004.gho.pok.ibm.com (b01ledav004.gho.pok.ibm.com [9.57.199.109]) by b01cxnp23032.gho.pok.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 076EkSb642729876 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Thu, 6 Aug 2020 14:46:28 GMT Received: from b01ledav004.gho.pok.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id EDCA1112061; Thu, 6 Aug 2020 14:46:27 +0000 (GMT) Received: from b01ledav004.gho.pok.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 0FF82112063; Thu, 6 Aug 2020 14:46:27 +0000 (GMT) Received: from [9.163.16.97] (unknown [9.163.16.97]) by b01ledav004.gho.pok.ibm.com (Postfix) with ESMTP; Thu, 6 Aug 2020 14:46:26 +0000 (GMT) Subject: Re: [PATCH v8] string: Adds tests for test-strncasecmp and test-strncpy To: Raoni Fassina Firmino References: <20200729182740.18447-1-rzinsly@linux.ibm.com> <20200731141439.jwqewilnsfrnw266@work-tp> Message-ID: <74cf61b4-3dbb-e86a-451f-7c6e2d13369d@linux.ibm.com> Date: Thu, 6 Aug 2020 11:46:25 -0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.8.0 MIME-Version: 1.0 In-Reply-To: <20200731141439.jwqewilnsfrnw266@work-tp> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-TM-AS-GCONF: 00 X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.235, 18.0.687 definitions=2020-08-06_09:2020-08-06, 2020-08-06 signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 malwarescore=0 phishscore=0 priorityscore=1501 impostorscore=0 mlxscore=0 bulkscore=0 mlxlogscore=999 suspectscore=0 spamscore=0 lowpriorityscore=0 adultscore=0 clxscore=1015 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2006250000 definitions=main-2008060100 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: , From: Raphael M Zinsly via Libc-alpha Reply-To: Raphael M Zinsly Cc: libc-alpha@sourceware.org Errors-To: libc-alpha-bounces@sourceware.org Sender: "Libc-alpha" Hi Raoni, Thanks for the review On 31/07/2020 11:14, Raoni Fassina Firmino wrote: > Hi Raphael, > > Sorry for my late review, hope it is useful. > > I Tested the patch on powerpc-linux-gnu (with kernel ppc64) on top of > master (0ad926f349) with no problems. > > >> diff --git a/string/test-strncasecmp.c b/string/test-strncasecmp.c > > OK. > > >> diff --git a/string/test-strncpy.c b/string/test-strncpy.c > ... > >> + const size_t maxoffset = 64; > > I am confused about the role of maxoffset == 64 and sizeof(CHAR), if the > intended value of 64 is for the buffer size in bytes or array elements. > But in any case I think It surely is not a problem to be larger than the > minimum 64 bytes, I am just pointing it out because I may be missing > something. > maxoffset is based on the usual register sizes as suggested here: https://sourceware.org/pipermail/libc-alpha/2020-June/114978.html > >> + /* Put s1 at the maxoffset from the edge of buf1's last page. */ >> + s1 = (CHAR *) buf1 + BUF1PAGES * page_size / sizeof(CHAR) - maxoffset; > > I don't understand how the position in the buffer is being calculated > here, IIUIC you want the last `maxoffset * sizeof(CHAR)` bytes at the > end of buf1, in this case `page_size / sizeof(CHAR)` doesn't make sense > for me, but my math may be off. > This was intended for test-wcsncpy, as `wchar_t` is bigger than `char` we need more space in the buffer to use the same test. Your suggestion makes sense, but the wmemset fails if s1 is placed with just `maxoffset * sizeof(CHAR)`, seems it is accessing the protected area. The page test at test-strnlen also uses `page_size / sizeof(CHAR)`. > >> + /* s2 needs room to put a string with size of maxoffset + 1 at s2 + >> + (maxoffset - 1). */ >> + s2 = (CHAR *) buf2 + page_size / sizeof(CHAR) - maxoffset * 2; > > Same as before, but also `maxoffset * 2` guarantees that only one of the > tests will touch the edge of buf2 page, I am not sure if this is the > intended case. > What do you mean by one of the tests? If you are referring to test-strncasecmp it's a different behavior, strncpy fills the `dest` string with NULL if the `src` is smaller than `N`, so it will reach the maxoffset+1 and the end of buf2. > >> + do_one_test (impl, (s2 + off2), (s1 + off1), maxoffset - off1 - 1, >> + maxoffset + 1); > > Reading do_one_test, it seems like len, here as `maxoffset - off1 - 1`, > should be `maxoffset - off1`, as it is supposed to be the size of src. > I know that because the trailing '\0' on src, this in practice will not > be a problem, but for a case when sizeof(src) and sizeof(dest) is the > same, do_one_test will ended up doing the last bit for len < n. > The len expected on do_one_test is the number of chars in `src`, if you take a look at test-stpncpy.c: #define STRNCPY_RESULT(dst, len, n) ((dst) + ((len) > (n) ? (n) : (len))) Counting the trailing '\0' will use it to compare with stpncpy's result thus getting a false fail. > nit, and totally optional, you don't need "()" around (s2 + off2) and > (s1 + off1), I am only mentioning because you didn't use it for the > other parameters. > > > > o/ > Raoni > Best Regards, -- Raphael Moreira Zinsly IBM Linux on Power Toolchain