netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Neil Horman <nhorman@tuxdriver.com>
To: Joe Perches <joe@perches.com>
Cc: Vlad Yasevich <vyasevich@gmail.com>,
	"David S. Miller" <davem@davemloft.net>,
	linux-sctp@vger.kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [RFC patch] sctp: sctp_generate_fwdtsn: Initialize sctp_fwdtsn_skip array, neatening
Date: Fri, 3 Jul 2015 07:51:05 -0400	[thread overview]
Message-ID: <20150703115105.GA3503@hmsreliant.think-freely.org> (raw)
In-Reply-To: <1435874096.2487.51.camel@perches.com>

On Thu, Jul 02, 2015 at 02:54:56PM -0700, Joe Perches wrote:
> It's not clear to me that the sctp_fwdtsn_skip array is
> always initialized when used.
> 
> It is appropriate to initialize the array to 0?
> 
> This patch initializes the array too 0 and moves the
> local variables into the blocks where used.
> 
> It also does some miscellaneous neatening by using
> continue; and unindenting the following block and
> using ARRAY_SIZE rather than 10 to decouple the
> array declaration size from a constant.
> ---
We don't set ftsn_skip_arr to a known value because we limit the amount of
elements that get read from it prior to those elements being set.  That is to
say, in our first use (the call to sctp_get_skip_pos), we pass the uninitialized
array, and the nskips value, which is initalized to 0.  Looking at the
definition of sctp_get_skip_pos, the for loop there becomes a nop, meaning the
uninitalized array is irrelevant, as we never visit any of its elements.
element zero is returned, and thats what the for_each loop in
sctp_generate_fwdtsn sets in that element of the array.  On the next iteration
of the for_each loop, we call sctp_get_skip_pos with nskips = 1, so only the
first element is visited, whcih was set by the previous loop iteration.


The rest of the cleanups look ok I think.  Can you tell me what you did to test
it?

Regards
Neil

  reply	other threads:[~2015-07-03 11:51 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-02 21:54 [RFC patch] sctp: sctp_generate_fwdtsn: Initialize sctp_fwdtsn_skip array, neatening Joe Perches
2015-07-03 11:51 ` Neil Horman [this message]
2015-07-03 16:41   ` Joe Perches
2015-07-06 13:43     ` Neil Horman

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=20150703115105.GA3503@hmsreliant.think-freely.org \
    --to=nhorman@tuxdriver.com \
    --cc=davem@davemloft.net \
    --cc=joe@perches.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-sctp@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=vyasevich@gmail.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).