Dear Stefano Babic, In message <1274439131-22807-1-git-send-email-sba...@denx.de> you wrote: > Add a sort of batch mode to fw_setenv, allowing to set > multiple variables in one shot, without updating the flash after > each set as now. It is added the possibility to pass
Thanks; that's a welcome improvement! > a config file with a list of pairs <variable, value> to be set, > separated by a TAB character. I think we should be less restrictive here. Please split at the first white space, and allow for multiple white spaces. > + /* Allocate enough place to the data string */ > for (i = 2; i < argc; ++i) { > char *val = argv[i]; > + if (!value) { > + value = (char *)malloc(len - strlen(name)); > + memset(value, 0, len - strlen(name)); Kaboom! when malloc() fails. > + tmpval = value; > + } > + if (i != 2) > + *tmpval++ = ' '; > + while (*val != '\0') > + *tmpval++ = *val++; > + } > + > + fw_set_single_var(name, value); Silent corruption of environment when malloc() failed. > + /* > + * Update CRC > + */ > + *environment.crc = crc32(0, (uint8_t *) environment.data, ENV_SIZE); > + > + /* write environment back to flash */ > + if (flash_io(O_RDWR)) { > + fprintf(stderr, "Error: can't write fw_env to flash\n"); > + return -1; ... > /* > * Update CRC > */ > - *environment.crc = crc32 (0, (uint8_t *) environment.data, ENV_SIZE); > + *environment.crc = crc32(0, (uint8_t *) environment.data, ENV_SIZE); > > /* write environment back to flash */ > - if (flash_io (O_RDWR)) { > + if (flash_io(O_RDWR)) { > fprintf (stderr, "Error: can't write fw_env to flash\n"); > return -1; We probably should not repeat this code, but factor it out. > +/* > + * Parse script to generate list of variables to set > + * The script file has a very simple format, as follows: > + * > + * Each line has a couple with name, value: > + * variable_name<TAB>variable_value Please make this: name<white space>value > + * Both variable_name and variable_value are interpreted as strings. > + * Any character after <TAB> and before ending \r\n is interpreted > + * as variable's value (no comment allowed on these lines !) > + * > + * Comments are allowed if the first character in the line is # > + * > + * Returns -1 and sets errno error codes: > + * 0 - OK > + * -1 - Error > + */ > +int fw_parse_script(char *fname, fw_env_list *list, int count) > +{ > + FILE *fp; > + int i = 0; > + char dump[128]; Ouch! That's short! Why do we need such an arbitrary limit? > + fp = fopen(fname, "r"); > + if (fp == NULL) > + return -1; How about a useful error message? > + while (fgets(dump, sizeof(dump), fp)) { > + /* Skip incomplete conversions and comment strings */ > + if (dump[0] == '#') > + continue; What are incomplete conversions, and where do they get skipped? And what happens with the rest of the input? > + list[i].name[0] = '\0'; > + list[i].value[0] = '\0'; > + > + val = strtok(dump, "\r\n"); > + if (!val) > + continue; > + > + name = strtok(dump, "\t"); > + if (!name) > + continue; > + > + strncpy(list[i].name, name, sizeof(list[i].name) - 1); Error message for too long names? > + val = strtok(NULL, "\t"); > + if (val) > + strncpy(list[i].value, val, sizeof(list[i].value) - 1); Error message for too long values? Hm... Isn't strtok() a bit of overkill here, when all we want to do is split at the first white space? > +typedef struct { > + char name[255]; > + char value[255]; > +} fw_env_list; Again we have arbitrary limits. And even different ones from the one above (128). > +static struct option long_options[] = { > + {"script", required_argument, NULL, 's'}, > + {NULL, 0, NULL, 0} > +}; The command should also accept '-' as file name, and then read from stdin. > + if (!script_file) { > + if (fw_setenv(argc, argv) != 0) > + return EXIT_FAILURE; > + } else { > + list = (fw_env_list *)malloc(MAX_SET_VARS * > + sizeof(fw_env_list)); > + count = fw_parse_script(script_file, > + list, MAX_SET_VARS); Kaboom! when malloc() failed. Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de "In matrimony, to hesitate is sometimes to be saved." - Butler _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot