Dear David Wagner,
In message 1312885889-20222-1-git-send-email-david.wagner@free-electrons.com you wrote:
This tool takes a key=value configuration file (same as would a `printenv' show) and generates the corresponding environnment image, ready to be flashed.
s/nnm/nm/
Signed-off-by: David Wagner david.wagner@free-electrons.com
Hi Mike,
This 3rd version should address what you pointed out.
If this is v3, then why don't you say so in the Subject: ???
Could you please explain which usage scenarios you have in mind for this tool? I would probably rather just load the text file you use as input, and run "env import -t" on it.
Checkpatch says:
total: 4 errors, 5 warnings, 228 lines checked
Please fix these.
In addition:
...
default:fprintf(stderr, "Wrong option -%c\n", option);usage(argv[0]);return EXIT_FAILURE;}- }
- /* Check datasize and allocate the data */
Please only one blank line in cases like this.
- /* envptr points to the beginning of the actual environment (after the
* crc and possible `redundant' bit */
inforrect multiline comment style.
- /* Open the configuration file ... */
- txt_filename = argv[optind];
- if (!txt_filename) {
fprintf(stderr, "Can't strdup() the configuration filename\n");return EXIT_FAILURE;
Where is there any strdup() involved ?
- txt_file = fopen(txt_filename, "rb");
- if (!txt_file) {
fprintf(stderr, "Can't open configuration file: %s\n", strerror(errno));
It would probably be very useful to also print the file name.
- /* ... and check it */
- ret = fstat(fileno(txt_file), &txt_file_stat);
Why don't you simply use mmap() ?
- for (i = 0 ; i < txt_file_stat.st_size ; i++)
if (envptr[i] == '\n')envptr[i] = '\0';
This is actually wrong. Envrionment variables can have embedded newlines.
- /*
* Make sure there is a final '\0' (necessary if the padding byte isn't* 0x00 or if there wasn't a newline at the end of the configuration
You did not take this into account in your file size check above, it seems.
* file) Also, don't add it if the configuration file size is exactly* the size of the environnment.
The double '\0' termination is _always_ needed.
Best regards,
Wolfgang Denk