PATCH: bug fix in IDE identification strings
Hi everybody,
The attached patch fixes a bug in the ide identification string copy.
CHANGELOG:
* Pierre AUBERT - 15 Mar 2004
- bug fix in ide identification.
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: u-boot-ide-ident.patch
Url: http://lists.denx.de/pipermail/u-boot/attachments/20040315/a0731539/attachment.txt
Quoted 2 lines · In message <[email protected]> you wrote:
The attached patch fixes a bug in the ide identification string copy.
I may be missing something - but why do we copy these strings in the first place? Who is using them? (and where?)
Best regards,
Wolfgang Denk
Signature · 5 lines
Software Engineering: Embedded and Realtime Systems, Embedded Linux Phone: (+49)-8142-4596-87 Fax: (+49)-8142-4596-88 Email: wd at denx.de "Data is a lot like humans: It is born. Matures. Gets married to other data, divorced. Gets old. One thing that it doesn't do is die. It has to be killed." - Arthur Miller
Quoted 12 lines · Wolfgang Denk
Quoted 3 lines · In message <[email protected]> you wrote:
The attached patch fixes a bug in the ide identification string copy.
I may be missing something - but why do we copy these strings in the first place? Who is using them? (and where?)
These strings are displayed by dev_print in disk/part.c
Quoted 6 lines
Best regards,
Wolfgang Denk
Best regards
Hi Wolfgang,
Quoted 6 lines · Wolfgang Denk
Quoted 2 lines · In message <[email protected]> you wrote:
The attached patch fixes a bug in the ide identification string copy.
I may be missing something - but why do we copy these strings in the first place? Who is using them? (and where?)
The block_dev_desc_t structure defines vendor[40], product[20] and revision[8] - same as it is defined in hd_driveid_t.
ident_cpy's description says that it will copy the string and terminate it, but it can't in the case where all 8 bytes of revision are occupied - since the buffers are equal size.
Therefore Pierre's patch is correct, but instead of hardcoded values 8, 40, 20 I would rather see sizeof (iop->fw_rev) etc.
I noticed the same thing today when I was testing the PCMCIA patch that I'm about to send for PXA.
Signature · 2 lines
Best Regards, Leon.
Hi,
Quoted 16 lines · Leon KUKOVEC
Hi Wolfgang,
Quoted 10 lines · Wolfgang Denk
Quoted 3 lines · In message <[email protected]> you wrote:
The attached patch fixes a bug in the ide identification string copy.
I may be missing something - but why do we copy these strings in the first place? Who is using them? (and where?)
Quoted 8 lines
The
block_dev_desc_tstructure defines vendor[40], product[20] and revision[8] - same as it is defined inhd_driveid_t.ident_cpy's description says that it will copy the string and terminate it, but it can't in the case where all 8 bytes of revision are occupied - since the buffers are equal size.
My hard disk has a 8 bytes revision string. In this case, u-boot crashes in init_part
because the block_read field of the block_dev_desc_t structure has been partially erased
by the ending null of the revision string. This crash didn't occured with the revison 1.5 of include/part.h
because the field removable was beetween fields revision and block_read.
Quoted 4 lines
Therefore Pierre's patch is correct, but instead of hardcoded values 8, 40, 20 I would rather see sizeof (
iop->fw_rev) etc.
The attached patch replaces my previous with this modification.
Quoted 5 lines
I noticed the same thing today when I was testing the PCMCIA patch that I'm about to send for PXA.
Best regards
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: u-boot-ide-ident.patch
Url: http://lists.denx.de/pipermail/u-boot/attachments/20040316/c9201d4a/attachment.txt
Dear Leon,
Quoted 3 lines · in message <[email protected]> you wrote:
Therefore Pierre's patch is correct, but instead of hardcoded values 8, 40, 20 I would rather see sizeof (
iop->fw_rev) etc.
This is what I implemented.
Thanks for pointing out.
Best regards,
Wolfgang Denk
Signature · 3 lines
Software Engineering: Embedded and Realtime Systems, Embedded Linux Phone: (+49)-8142-4596-87 Fax: (+49)-8142-4596-88 Email: wd at denx.de 1 1 was a race-horse, 2 2 was 1 2. When 1 1 1 1 race, 2 2 1 1 2.
Hi again Wolfgang,
The IDE ident problem overflow problem isn't solved in the CVS. If a string has trailing whitespaces, they're not always suppressed. It depends on the character following the end of the source string.
The attached patch fixes that.
CHANGELOG:
* Patch by Pierre Aubert 18 Mar 2004
- Fix string cleaning in IDE identification
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: u-boot-ide2.patch
Url: http://lists.denx.de/pipermail/u-boot/attachments/20040318/3e12406e/attachment.txt
Quoted 4 lines · In message <[email protected]> you wrote:
CHANGELOG:
* Patch by Pierre Aubert 18 Mar 2004
- Fix string cleaning in IDE identification
Thanks, added.
Best regards,
Wolfgang Denk
Signature · 6 lines
Software Engineering: Embedded and Realtime Systems, Embedded Linux
Phone: (+49)-8142-4596-87 Fax: (+49)-8142-4596-88 Email: wd at denx.de
You young'uns. That was *long* before AltaVista, DejaNews, or even
(gasp) the *Web*! In fact, we typed that thread on steam-powered card
punchers, and shipped it around via Pony Express.
-- Randal Schwartz in <[email protected]>Quoted 6 lines · In message <[email protected]> you wrote:
The attached patch fixes a bug in the ide identification string copy.
CHANGELOG:
* Pierre AUBERT - 15 Mar 2004
- bug fix in ide identification.
Thanks.
I chose to implement this differently (keeping the sizeof() which I like much better than magic hardwired sizes). Now ident_cpy() knows that the "len" given includes the terminating NUL byte.
Best regards,
Wolfgang Denk
Signature · 4 lines
Software Engineering: Embedded and Realtime Systems, Embedded Linux Phone: (+49)-8142-4596-87 Fax: (+49)-8142-4596-88 Email: wd at denx.de "He was so narrow minded he could see through a keyhole with both eyes ..."