Messages in this thread | | | Date | Wed, 9 Feb 2011 09:27:07 +0100 | Subject | Re: [PATCH 4/5] staging: ft1000: Fix coding style in write_blk_fifo() function. | From | Belisko Marek <> |
| |
On Tue, Feb 8, 2011 at 5:35 PM, Dan Carpenter <error27@gmail.com> wrote: > 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. You're right. Today will make test when remove all opaque code. Anyway at the end file position is moved in that way: *pUsFile = *pUsFile + (word_length << 1); *pUcFile = *pUcFile + (word_length << 2);
So short pointer multiplied by 2 and char pointer by 4 with word_length. So my assume all check and byte_length increasing is not correct (will see what test shows). > > regards, > dan carpenter > > >
regards,
marek
-- as simple and primitive as possible ------------------------------------------------- Marek Belisko - OPEN-NANDRA Freelance Developer
Ruska Nova Ves 219 | Presov, 08005 Slovak Republic Tel: +421 915 052 184 skype: marekwhite icq: 290551086 web: http://open-nandra.com -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |