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.
_______________________________________________ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev