On Mon, 2018-10-29 at 17:08 -0400, Theodore Y. Ts'o wrote:
> On Mon, Oct 29, 2018 at 09:26:43AM -0700, Bart Van Assche wrote:
> >
> > Have you considered to change the data type of 'len' from size_t into
> > unsigned long
> > instead of inserting this cast? That would make it clear that no integer
> > truncation
> > happens in the iov.append() call.
>
> Well the potential integer truncation that could happen is here:
>
> case 'l': len = strtoul(optarg, NULL, 0); break;
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?
> I have a sneaking suspicion that ib_srp has zero change of
> working on 32-bit systems (which is what uses this function) so this
> was more about making sure blktests would build when I'm building the
> 32-bit version of my test appliance VM.
What makes you think that ib_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.
> (For that matter, I've been banging my head against a brick wall
> trying to make the srp tests work in either a KVM or GCE environment,
> even with a 64-bit VM, but that's a different issue. It's not high
> priority for me, at the moment, but eventually I would like the
> {kvm,gce,android}-xfstest test appliance VM's generated by the
> 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?
> What offends me more is that later on there are scopes when len gets
> shadowed with a "ssize_t len;" definition --- I whould have thought
> gcc or clang would have complained bitterly about that, but I guess
> 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.
