On 4/17/23 22:17, Ralph Siemsen wrote:
On Mon, Apr 17, 2023 at 07:23:46PM +0200, Marek Vasut wrote:
On 3/8/23 21:26, Ralph Siemsen wrote:
+ spkgimage.o \
Maybe just call the file renesas_spkgimage.o so its clear which SoC/vendor this file is associtated with.
Okay, will do.
+static struct spkg_file out_buf;
+static uint32_t padding;
Is this padding here and the padding in struct config_file below different padding ? Can we get rid of these static global variables ?
I will give it a try.
+static int check_range(const char *name, int val, int min, int max) +{ + if (val < min) { + fprintf(stderr, "Warning: param '%s' adjusted to min %d\n", + name, min); + val = min; + }
+ if (val > max) { + fprintf(stderr, "Warning: param '%s' adjusted to max %d\n", + name, max); + val = max; + }
There is a macro clamp() which implements range limiting .
Thanks for pointing that out. However I think there is value in the diagnostic print when the value is clamped. Ideally it should help the user to fix their invoking script/binman/etc.
Of course, I could call clamp() and check if the value differs, but that seems just as complex as the check_range().
ACK
+ while (fgets(line, sizeof(line), fcfg)) { + line_num += 1;
+ /* Skip blank lines and comments */ + if (line[0] == '\n' || line[0] == '#') + continue;
+ /* Strip the trailing newline */ + len = strlen(line); + if (line[len - 1] == '\n') + line[--len] = 0;
Use len - 1 here too to avoid confusion ?
Old habit. I always try to update the length in sync with modifying the string. If done as a separate line/statement, it is more likely to be lost during subsequent modifications.
In this case I do not need "len" at all, so I could just do:
Nice