qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH] e1000: cleanup process_tx_desc


From: Luigi Rizzo
Subject: Re: [Qemu-devel] [PATCH] e1000: cleanup process_tx_desc
Date: Tue, 4 Jun 2013 09:54:17 +0200

On Tue, Jun 4, 2013 at 9:34 AM, Andrew Jones <address@hidden> wrote:


----- Original Message -----
> On 06/03/2013 10:20 AM, Andrew Jones wrote:
> > Coverity complains about two overruns in process_tx_desc(). The
> > complaints are false positives, but we might as well eliminate
> > them. The problem is that "hdr" is defined as an unsigned int,
> > but then used to offset an array of size 65536, and another of
> > size 256 bytes. hdr will actually never be greater than 255
> > though, as it's assigned only once and to the value of
> > tp->hdr_len, which is an uint8_t. This patch simply gets rid of
> > hdr, replacing it with tp->hdr_len, which makes it consistent
> > with all other tp member use in the function.
> >
> > Signed-off-by: Andrew Jones <address@hidden>
> > ---
> >  hw/net/e1000.c | 16 ++++++++--------
> >  1 file changed, 8 insertions(+), 8 deletions(-)
> >
>
> The logic looks sound, but checkpatch detects some style issues. See below.

...
> Although the style issues were present to begin with, we may as well take
> the opportunity to fix them.

Running checkpatch on the file yields

142 errors, 41 warnings

I could send a v2 that fixes the 1 error and 2 warnings found in the context
of this patch, but why? It's out of the scope of the patch (although I did
use "cleanup" in the summary...), and it would hardly make a dent in this
file's problems.


Indeed. I find it slightly annoying (and a waste of everyone's time)
that patches are bounced on issues that they are not responsible for.
(this happens for several other opensource projects I have been
involved with).

I think that a much more effective approach would be to take the patch
(on the grounds that it improves the codebase),
and then if the committer feels like fixing the pre-existing
style issue he can do it separately, or make a comment in the
commit log if he has no time (and by the same reasoning, the original
submitter may be short of time).

cheers
luigi

Many projects i have been involved with have this

drew

>
> Sincerely,
>
> Jesse Larrew
> Software Engineer, KVM Team
> IBM Linux Technology Center
> Phone: (512) 973-2052 (T/L: 363-2052)
> address@hidden
>
>




--
-----------------------------------------+-------------------------------
 Prof. Luigi RIZZO, address@hidden  . Dip. di Ing. dell'Informazione
 http://www.iet.unipi.it/~luigi/        . Universita` di Pisa
 TEL      +39-050-2211611               . via Diotisalvi 2
 Mobile   +39-338-6809875               . 56122 PISA (Italy)
-----------------------------------------+-------------------------------


reply via email to

[Prev in Thread] Current Thread [Next in Thread]