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:

quoted

From: Steve Wise <redacted>
Date: Wed, 19 Jun 2013 21:19:13 -0500

quoted

On 6/19/2013 8:01 PM, David Miller wrote:

quoted

From: Vipul Pandya <redacted>
Date: Wed, 12 Jun 2013 17:11:38 +0530

quoted

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 <redacted>

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 linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html