Modify

Opened 10 years ago

Closed 10 years ago

#2483 closed enhancement (fixed)

fix portability bugs and add features to motorola-bin

Reported by: hugh@… Owned by: developers
Priority: normal Milestone:
Component: toolchain Version:
Keywords: Cc: hugh@…

Description

Here is a version of ttps://svn.openwrt.org/openwrt/trunk/tools/firmware-utils/src/motorola-bin.c with a bunch of small fixes and improvements.

  • I've tested this only very lightly.
  • I know of no documentation for motorola-bin.c so my changes don't make it match the non-existant) specifications any better.
  • I'm a bit of a perfectionist. Perhaps nobody cares about these changes.

Recommended changes:

  • several places where a 32-bit value is desired, type "unsigned long" or "unsigned int" is used. The C standard does not promise that this will work. The right type is uint32_t. CRC32 values are 32 bits. So is the flags field in the .trx header.
  • a failing malloc returns NULL, not -1. But the malloc in init_crc32 doesn't seem to be better than static allocation anyway.
  • The usage message states that the operands are optional. But the code demands that the operands all be present: exactly 3 of them. Fix the usage message.
  • the length of the input file is the return value of lseek. The type of this is off_t, not unsigned long. The unsigned long typing will mask any failure (an unsigned long can never be less than 0).
  • struct motorola has a third field "trx" that is a char*. This is just wrong. Perhaps char[0] would be correct but C does not allow it. Best to just get rid of that field.
  • the calls of ntohl applied to the flags literals ought to be calls to htonl. This makes not actual difference.
  • diagnostics should go to stderr, not stdout.
  • crc of buffer should be unsigned char *, not char *
  • where possible, global variables and functions should be limited in scope to file static.
  • the naming "trxfile" and "binfile" confuses me. trxfile is the input file, without the trx header; binfile is the output file, with the trx header. Is there a good explanation of terminology?
  • the operand to specify the device could name the device rather than use a cryptic number to refer to it.
  • add a --strip flag to reverse the process, first checking if the header is correct

The diff is larger than the revised source code so I have included the whole file as well as the diff.

Attachments (2)

motorola-bin.c.diff (7.4 KB) - added by hugh@… 10 years ago.
diff
motorola-bin.c (5.7 KB) - added by hugh@… 10 years ago.
revised source file

Download all attachments as: .zip

Change History (3)

Changed 10 years ago by hugh@…

diff

Changed 10 years ago by hugh@…

revised source file

comment:1 Changed 10 years ago by nbd

  • Resolution set to fixed
  • Status changed from new to closed

added in [9434]

Add Comment

Modify Ticket

Action
as closed .
The resolution will be deleted. Next status will be 'reopened'.
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.