netdev - Re: [PATCH V2 0/4] Add IPv6 support for iWARP

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <51D2779D.6060100@chelsio.com>
Date:	Tue, 02 Jul 2013 12:17:57 +0530
From:	Vipul Pandya <vipul@...lsio.com>
To:	David Miller <davem@...emloft.net>
CC:	SWise OGC <swise@...ngridcomputing.com>,
	"linux-rdma@...r.kernel.org" <linux-rdma@...r.kernel.org>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"roland@...estorage.com" <roland@...estorage.com>,
	Divy Le Ray <divy@...lsio.com>,
	Dimitrios Michailidis <dm@...lsio.com>,
	"roland@...nel.org" <roland@...nel.org>,
	"sean.hefty@...el.com" <sean.hefty@...el.com>,
	"hal.rosenstock@...il.com" <hal.rosenstock@...il.com>,
	Tom Tucker <tom@...ngridcomputing.com>,
	"faisal.latif@...el.com" <faisal.latif@...el.com>,
	"akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
	"sasha.levin@...cle.com" <sasha.levin@...cle.com>,
	Nirranjan Kirubaharan <nirranjan@...lsio.com>
Subject: Re: [PATCH V2 0/4] Add IPv6 support for iWARP



On 20-06-2013 19:07, Vipul Pandya wrote:
> 
> 
> On 20-06-2013 09:38, David Miller wrote:
>> From: Steve Wise <swise@...ngridcomputing.com>
>> Date: Wed, 19 Jun 2013 21:19:13 -0500
>>
>>> On 6/19/2013 8:01 PM, David Miller wrote:
>>>> From: Vipul Pandya <vipul@...lsio.com>
>>>> Date: Wed, 12 Jun 2013 17:11:38 +0530
>>>>
>>>>> We have included all the maintainers of respective drivers. Kindly
>>>>> review the change and let us know in case of any review comments.
>>>> I have not seen anyone review v2 of this patch series.
>>>>
>>>
>>> Reviewed-by: Steve Wise <swise@...ngridcomputing.com>
>>
>> You wrote the first patch, and I bet you didn't even read the code in
>> the cxgb4 driver.  So your review is sort of pointless... UNLESS you
>> spotted the obvious bugs in these changes, that would have been
>> interesting.
>>
>> Because NOBODY, and I mean NOBODY, even looked at the build of the
>> cxgb4 changes.
>>
>> Tell me what this does:
>>
>>  	struct tid_info *t = dev->rdev.lldi.tids;
>>  	int status = GET_AOPEN_STATUS(ntohl(rpl->atid_status));
>> +	struct sockaddr_in *la = (struct sockaddr_in *)&ep->com.local_addr;
>> +	struct sockaddr_in *ra = (struct sockaddr_in *)&ep->com.remote_addr;
>> +	struct sockaddr_in6 *la6 = (struct sockaddr_in6 *)&ep->com.local_addr;
>> +	struct sockaddr_in6 *ra6 = (struct sockaddr_in6 *)&ep->com.remote_addr;
>> +
>> +
>>  
>>  	ep = lookup_atid(t, atid);
>>
>> Dereferencing 'ep' before initializing it.
>>
>> The compiler complains loudly about this, therefore nobody even looked at
>> the build logs from these changes before submitting them to me.
>>
>> That translates to "don't care", and if the people submitting this
>> code don't care why should I?
>>
>> Sorry, not impressed.  I'm seriously going to take my time reviewing
>> any future submissions of these changes, because it's obvious that
>> even the people writing and submitting this code DO NOT CARE.
>>
> 
> I am really very sorry for this. Somehow my compiler is not giving me
> any warnings for this. My compiler is gcc 4.4.6 20120305 (Red Hat
> 4.4.6-4). Previously also once it has happened that my compiler did not
> give any warning but your build environment caught. Is there any special
> gcc option I have to pass with make command for this? I am following the
> checklist mentioned in Documentation/SubmitChecklist file.
> 
> We always make sure all our drivers are building cleanly before
> submitting the drivers. We also have unit tested this code. However the
> problematic code gets executed only in error path hence could not catch
> during unit testing.
> 
> I will resubmitt the series with the changes. Your review comments are
> very valuable for us.
> 

I upgraded my GCC version from 4.4.6-4 to 4.8.1 and after that I am able
to see that warning. I will resubmit this series soon.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html