Re: [Qemu-devel] [PATCH v2 01/13] Use unsigned types for the 'len' argument of all memory read/write functions

[Martin Galvan]

On Tue, Mar 1, 2016 at 1:05 PM, Eric Blake <address@hidden> wrote:
> On 03/01/2016 08:57 AM, Martin Galvan wrote:
>
> Missing a S-o-b line; we cannot accept patches unless you sign them:
> http://wiki.qemu.org/Contribute/SubmitAPatch
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/SubmittingPatches?id=f6f94e2a#n297

Oh, I forgot that. I did have a s-o-b in v1, I just forgot to add it here.

> Subject line is long, and lacking a 'topic: summary' prefix.  I'd suggest:
>
> mem: use unsigned 'len' for read/write
>
> Then give more details, if needed, in the body of the commit message.

Yeah, I thought it was too long myself. Will do.

> Also, your threading didn't work right.  Patch 1/13 was sent with:
>
> References:
> <address@hidden>
>
> but with no In-Reply-To:. Meanwhile, the 00/13 cover letter was sent with:
>
> Message-Id:
> <address@hidden>
>
> which is subtly different from the references of all the other patches,
> making it appear as separate threads.

Yeah, I think I messed that up. Sorry.

>>      FILE *f;
>> -    uint32_t l;
>> +    size_t l;
>
> These are both unsigned types.  So based on the subject line alone, this
> doesn't fit in the patch, unless the commit message body goes into more
> details on why you are changing the size of the type, and not just the
> signedness.

You're right, I recall changing that for size correctness, rather than
signedness. I'll mention in the commit message that I also fixed those
issues in a couple of places.

> (adding an appropriate prefix for each maintainer that you are targetting 
> will help).

I actually thought of that, but the files grouped into each e-mail
weren't necessarily related (e.g. Paolo's include some stuff in exec/
and kvm-all.c). I'll see what I can do, though.

Before I re-send them, however, it'd be nice if the patches themselves
could be reviewed.