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
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
