From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 81415 invoked by alias); 14 Nov 2016 17:54:35 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 81396 invoked by uid 89); 14 Nov 2016 17:54:33 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.5 required=5.0 tests=AWL,BAYES_00,KAM_LAZY_DOMAIN_SECURITY,RCVD_IN_DNSWL_LOW autolearn=no version=3.3.2 spammy=luis, Luis, Machado, machado X-HELO: mx0a-001b2d01.pphosted.com Received: from mx0b-001b2d01.pphosted.com (HELO mx0a-001b2d01.pphosted.com) (148.163.158.5) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 14 Nov 2016 17:54:32 +0000 Received: from pps.filterd (m0098419.ppops.net [127.0.0.1]) by mx0b-001b2d01.pphosted.com (8.16.0.17/8.16.0.17) with SMTP id uAEHrwGi000828 for ; Mon, 14 Nov 2016 12:54:30 -0500 Received: from e06smtp07.uk.ibm.com (e06smtp07.uk.ibm.com [195.75.94.103]) by mx0b-001b2d01.pphosted.com with ESMTP id 26qefjc5em-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for ; Mon, 14 Nov 2016 12:54:30 -0500 Received: from localhost by e06smtp07.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 14 Nov 2016 17:54:28 -0000 Received: from d06dlp02.portsmouth.uk.ibm.com (9.149.20.14) by e06smtp07.uk.ibm.com (192.168.101.137) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Mon, 14 Nov 2016 17:54:27 -0000 Received: from b06cxnps3074.portsmouth.uk.ibm.com (d06relay09.portsmouth.uk.ibm.com [9.149.109.194]) by d06dlp02.portsmouth.uk.ibm.com (Postfix) with ESMTP id EDA50219005E for ; Mon, 14 Nov 2016 17:53:39 +0000 (GMT) Received: from d06av03.portsmouth.uk.ibm.com (d06av03.portsmouth.uk.ibm.com [9.149.37.213]) by b06cxnps3074.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id uAEHsQP842074208 for ; Mon, 14 Nov 2016 17:54:26 GMT Received: from d06av03.portsmouth.uk.ibm.com (localhost [127.0.0.1]) by d06av03.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id uAEHsPxe009576 for ; Mon, 14 Nov 2016 10:54:26 -0700 Received: from oc1027705133.ibm.com (dyn-9-152-212-182.boeblingen.de.ibm.com [9.152.212.182]) by d06av03.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVin) with ESMTP id uAEHsLl5009557 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Mon, 14 Nov 2016 10:54:25 -0700 From: Andreas Arnez To: Luis Machado Cc: Subject: Re: [PATCH 2/3] Fix copy_bitwise() References: <1479135786-31150-1-git-send-email-arnez@linux.vnet.ibm.com> <1479135786-31150-3-git-send-email-arnez@linux.vnet.ibm.com> Date: Mon, 14 Nov 2016 17:54:00 -0000 In-Reply-To: (Luis Machado's message of "Mon, 14 Nov 2016 09:38:06 -0600") User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 16111417-0028-0000-0000-0000024B18C4 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 16111417-0029-0000-0000-0000213574CC Message-Id: X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2016-11-14_10:,, signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=0 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1609300000 definitions=main-1611140360 X-IsSubscribed: yes X-SW-Source: 2016-11/txt/msg00359.txt.bz2 On Mon, Nov 14 2016, Luis Machado wrote: > On 11/14/2016 09:02 AM, Andreas Arnez wrote: [...] >> + dest += dest_offset / 8; >> + dest_offset %= 8; >> + source += source_offset / 8; >> + source_offset %= 8; > > Are you sure you will always have non-zero source_offset and dest_offset > when explicitly dividing them by 8? If i were to feed (or GDB, in some > erroneous state) invalid data to the function, this would likely crash? > > There are other cases of explicit / operations. No, copy_bitwise should work fine with source_offset and dest_offset set to zero. Where do you think it would crash? [...] >> + /* Fill BUF with DEST_OFFSET bits from the destination and 8 - >> + SOURCE_OFFSET bits from the source. */ >> + buf = *(bits_big_endian ? source-- : source++) >> source_offset; > > Maybe it's just me, but having constructs like the above don't help much > performance-wise and make the code slightly less readable. Should we > expand this further? There are multiple occurrences of this. Well, I've tried a few different ways and found this approach actually the easiest to read, for my taste. For instance, it makes the multiple occurrences easy to recognize -- as you pointed out ;-) Of course, if people feel that this post-decrement/increment pattern really hurts readability, I can provide a more "stretched" form instead. > Also, should we harden the method to prevent dereferencing NULL source or > dest? I wouldn't consider that necessary, but I'm open for other opinions. [...] > Otherwise it looks good to me. Thanks! -- Andreas