From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pg1-f194.google.com ([209.85.215.194]:42797 "EHLO mail-pg1-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727265AbeJ3GXI (ORCPT ); Tue, 30 Oct 2018 02:23:08 -0400 Received: by mail-pg1-f194.google.com with SMTP id i4-v6so4526652pgq.9 for ; Mon, 29 Oct 2018 14:32:38 -0700 (PDT) Message-ID: <1540848754.196084.105.camel@acm.org> Subject: Re: [PATCH blktests] Fix build failure for discontiguous-io on 32-bit platforms From: Bart Van Assche To: "Theodore Y. Ts'o" Cc: linux-block@vger.kernel.org Date: Mon, 29 Oct 2018 14:32:34 -0700 In-Reply-To: <20181029210822.GA15839@thunk.org> References: <20181029161534.7564-1-tytso@mit.edu> <1540830403.196084.50.camel@acm.org> <20181029210822.GA15839@thunk.org> Content-Type: text/plain; charset="UTF-7" Mime-Version: 1.0 Sender: linux-block-owner@vger.kernel.org List-Id: linux-block@vger.kernel.org On Mon, 2018-10-29 at 17:08 -0400, Theodore Y. Ts'o wrote: +AD4 On Mon, Oct 29, 2018 at 09:26:43AM -0700, Bart Van Assche wrote: +AD4 +AD4 +AD4 +AD4 Have you considered to change the data type of 'len' from size+AF8-t into unsigned long +AD4 +AD4 instead of inserting this cast? That would make it clear that no integer truncation +AD4 +AD4 happens in the iov.append() call. +AD4 +AD4 Well the potential integer truncation that could happen is here: +AD4 +AD4 case 'l': len +AD0 strtoul(optarg, NULL, 0)+ADs break+ADs If the data type of 'len' would be changed into unsigned long, how could that assignment cause integer truncation since strtoul() returns an unsigned long value? +AD4 I have a sneaking suspicion that ib+AF8-srp has zero change of +AD4 working on 32-bit systems (which is what uses this function) so this +AD4 was more about making sure blktests would build when I'm building the +AD4 32-bit version of my test appliance VM. What makes you think that ib+AF8-srp won't work on 32-bit systems? I don't think that anyone uses this driver on a 32-bit system. I'm not aware of any aspect of that driver that restricts it to 64-bit systems only either. +AD4 (For that matter, I've been banging my head against a brick wall +AD4 trying to make the srp tests work in either a KVM or GCE environment, +AD4 even with a 64-bit VM, but that's a different issue. It's not high +AD4 priority for me, at the moment, but eventually I would like the +AD4 +AHs-kvm,gce,android+AH0--xfstest test appliance VM's generated by the +AD4 xfstests-bld repo to be able to run all of blktests.) Can you be more specific about what didn't work? Were you unable to start the tests or did a particular test fail? +AD4 What offends me more is that later on there are scopes when len gets +AD4 shadowed with a +ACI-ssize+AF8-t len+ADsAIg definition --- I whould have thought +AD4 gcc or clang would have complained bitterly about that, but I guess +AD4 not. I agree that it would be an improvement to get rid of variable shadowing. Once that has been done the -Wshadow compiler flag can be added. I will look into this if nobody else starts looking into this soon. Bart.