From: Steve Wise <swise@opengridcomputing.com>
To: David Miller <davem@davemloft.net>
Cc: vipul@chelsio.com, linux-rdma@vger.kernel.org,
netdev@vger.kernel.org, roland@purestorage.com, divy@chelsio.com,
dm@chelsio.com, roland@kernel.org, sean.hefty@intel.com,
hal.rosenstock@gmail.com, tom@opengridcomputing.com,
faisal.latif@intel.com, akpm@linux-foundation.org,
sasha.levin@oracle.com, nirranjan@chelsio.com
Subject: Re: [PATCH V2 0/4] Add IPv6 support for iWARP
Date: Thu, 20 Jun 2013 08:33:05 -0500 [thread overview]
Message-ID: <51C30491.1050507@opengridcomputing.com> (raw)
In-Reply-To: <20130619.210805.224358826374432656.davem@davemloft.net>
On 6/19/2013 11:08 PM, David Miller wrote:
> From: Steve Wise <swise@opengridcomputing.com>
> Date: Wed, 19 Jun 2013 21:19:13 -0500
>
>> On 6/19/2013 8:01 PM, David Miller wrote:
>>> From: Vipul Pandya <vipul@chelsio.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@opengridcomputing.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.
>
We do care. We screwed up.
next prev parent reply other threads:[~2013-06-20 13:33 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-06-12 11:41 [PATCH V2 0/4] Add IPv6 support for iWARP Vipul Pandya
2013-06-12 11:41 ` [PATCH V2 2/4] cxgb4: Add routines to create and remove listening IPv6 servers Vipul Pandya
[not found] ` <1371037302-3586-1-git-send-email-vipul-ut6Up61K2wZBDgjK7y7TUQ@public.gmane.org>
2013-06-12 11:41 ` [PATCH V2 1/4] RDMA/cma: Add IPv6 support for iWARP Vipul Pandya
[not found] ` <1371037302-3586-2-git-send-email-vipul-ut6Up61K2wZBDgjK7y7TUQ@public.gmane.org>
2013-06-12 19:56 ` Nikolova, Tatyana E
[not found] ` <13AA599688F47243B14FCFCCC2C803BB0D814E35-AtyAts71sc864kNsxIetb7fspsVTdybXVpNB7YpNyf8@public.gmane.org>
2013-06-12 20:33 ` David Miller
2013-06-12 11:41 ` [PATCH V2 3/4] cxgb4: Add CLIP support to store compressed IPv6 address Vipul Pandya
2013-06-12 11:41 ` [PATCH V2 4/4] RDMA/cxgb4: Add support for active and passive open connection with " Vipul Pandya
2013-06-20 1:01 ` [PATCH V2 0/4] Add IPv6 support for iWARP David Miller
[not found] ` <20130619.180120.1989500368231967461.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
2013-06-20 2:19 ` Steve Wise
[not found] ` <51C266A1.2050003-7bPotxP6k4+P2YhJcF5u+vpXobYPEAuW@public.gmane.org>
2013-06-20 4:08 ` David Miller
2013-06-20 13:33 ` Steve Wise [this message]
2013-06-20 13:37 ` Vipul Pandya
[not found] ` <51C3058D.205-ut6Up61K2wZBDgjK7y7TUQ@public.gmane.org>
2013-07-02 6:47 ` Vipul Pandya
[not found] ` <51D2779D.6060100-ut6Up61K2wZBDgjK7y7TUQ@public.gmane.org>
2013-07-02 7:07 ` David Miller
2013-07-02 7:41 ` Vipul Pandya
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=51C30491.1050507@opengridcomputing.com \
--to=swise@opengridcomputing.com \
--cc=akpm@linux-foundation.org \
--cc=davem@davemloft.net \
--cc=divy@chelsio.com \
--cc=dm@chelsio.com \
--cc=faisal.latif@intel.com \
--cc=hal.rosenstock@gmail.com \
--cc=linux-rdma@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=nirranjan@chelsio.com \
--cc=roland@kernel.org \
--cc=roland@purestorage.com \
--cc=sasha.levin@oracle.com \
--cc=sean.hefty@intel.com \
--cc=tom@opengridcomputing.com \
--cc=vipul@chelsio.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).