On Thursday 02 February 2012 04:11:27 Chander Kashyap wrote:
--- /dev/null +++ b/board/samsung/smdk5250/tools/mkexynos_image.c
tools should be in tools/. there are already plenty of examples in there (see tools/msxboot.c as the first example i found by reading tools/Makefile).
+int main(int argc, char **argv) +{ ...
- unsigned char buffer[BUFSIZE] = {0};
this is an implicit memset() and from what i can see in the code, useless. you read() the entire buffer, so there's no need to initialize it.
- if (argc != 3) {
printf(" %d Wrong number of arguments\n", argc);
this should tell the user how to use the tool. fprintf(stderr, "Usage: %s <infile> <outfile>\n", argv[0]);
if (ifd)close(ifd);
this if() is wrong (0 is a valid fd) and useless (you already abort if ifd did not succeed). just delete the if statement.
- len = lseek(ifd, 0, SEEK_END);
- lseek(ifd, 0, SEEK_SET);
lazy man's stat() :P. just use stat(). and change the type of "len" to "off_t".
- count = (len < CHECKSUM_OFFSET) ? len : CHECKSUM_OFFSET;
- if (read(ifd, buffer, count) != count) {
count should be a ssize_t. although, this doesn't handle partial interrupted reads, so i wonder if this could shouldn't just be changed to use stdio fopen/fread. probably would be simpler that way too.
if (ifd)close(ifd);if (ofd)close(ofd);
these if checks are wrong for the same reason mentioned above
- unsigned long checksum = 0;
...
- for (i = 0, checksum = 0; i < CHECKSUM_OFFSET; i++)
checksum += buffer[i];- memcpy(&buffer[CHECKSUM_OFFSET], &checksum, sizeof(checksum));
pretty sure this fails if this tool is run on a big-endian machine, as well as 64bit vs 32bit. change the type of "checksum" to uint32_t, then use something like: put_unaligned_le32(checksum, &buffer[CHECKSUM_OFFSET]);
- if (write(ofd, buffer, BUFSIZE) != BUFSIZE) {
same issues as the read() mentioned above
if (ifd)close(ifd);if (ofd)close(ofd);
same bad if() logic
- if (ifd)
close(ifd);- if (ofd)
close(ofd);
same bad if() logic -mike