LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Dan Carpenter <error27@gmail.com>
To: Belisko Marek <marek.belisko@gmail.com>
Cc: Marek Belisko <marek.belisko@open-nandra.com>,
	gregkh@suse.de, devel@driverdev.osuosl.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 4/5] staging: ft1000: Fix coding style in write_blk_fifo() function.
Date: Tue, 8 Feb 2011 19:35:31 +0300	[thread overview]
Message-ID: <20110208163531.GO4384@bicker> (raw)
In-Reply-To: <AANLkTi=ygn3Go7kCYe1fAo6OrKQaVVGaV_=xMgxM=u5s@mail.gmail.com>

On Tue, Feb 08, 2011 at 02:40:49PM +0100, Belisko Marek wrote:
> On Wed, Jan 26, 2011 at 3:30 PM, Dan Carpenter <error27@gmail.com> wrote:
> > Also when it does:
> >        memcpy(ft1000dev->tx_buf, *pUcFile, byte_length);
> >
> > That should probably be:
> >        memcpy(ft1000dev->tx_buf, *pUcFile, word_length * 4);
> No this shouldn't because before you have additional check:
> if (byte_length && ((byte_length % 64) == 0))
>         byte_length += 4;
> 
> if (byte_length < 64)
>         byte_length = 68;
> So in my opinion byte_length should stay.

Yes.  We make byte_length longer than the caller intended.  The caller
knows the size of the source buffer.  We have to pad the length of the
other buffer, but we should fill up the last part with zeroes instead
of reading past the end of the source buffer.

(I am not very familiar with the code and I haven't looked outside this
function, so I may be wrong).

Also I really bet that the thing where byte_length can't be a multiple
of 64 is bogus.  I've never heard of anything with a requirement like
that.

regards,
dan carpenter



  reply	other threads:[~2011-02-08 16:36 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-01-26 11:49 [PATCH 1/5] staging: ft1000: Remove unused variables Marek Belisko
2011-01-26 11:49 ` [PATCH 2/5] staging: ft1000: Remove unnecessary assignment Marek Belisko
2011-01-26 11:49 ` [PATCH 3/5] staging: ft1000: Create common function for buffers check Marek Belisko
2011-01-26 11:49 ` [PATCH 4/5] staging: ft1000: Fix coding style in write_blk_fifo() function Marek Belisko
2011-01-26 13:07   ` Dan Carpenter
2011-01-26 13:34     ` Belisko Marek
2011-01-26 13:43     ` Dan Carpenter
2011-01-26 13:56       ` Belisko Marek
2011-01-26 14:30     ` Dan Carpenter
2011-02-08 13:40       ` Belisko Marek
2011-02-08 16:35         ` Dan Carpenter [this message]
2011-02-09  8:27           ` Belisko Marek
2011-02-09 10:07           ` Belisko Marek
2011-02-09 11:08             ` Dan Carpenter
2011-02-09 11:16               ` Belisko Marek
2011-01-26 11:49 ` [PATCH 5/5] staging: ft1000: Fix indentation in scram_dnldr() function Marek Belisko

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=20110208163531.GO4384@bicker \
    --to=error27@gmail.com \
    --cc=devel@driverdev.osuosl.org \
    --cc=gregkh@suse.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=marek.belisko@gmail.com \
    --cc=marek.belisko@open-nandra.com \
    --subject='Re: [PATCH 4/5] staging: ft1000: Fix coding style in write_blk_fifo() function.' \
    /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

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).