Hi, On 1/8/19 11:31 AM, Markus Armbruster wrote: > I'd leave it hard-coded. Replacing a few trivial defines by an arguably > less trivial script doesn't feel like an improvement. In this case, it > doesn't even save lines.
As I've said, I'm fine with that. The autogeneration sounds the right thing to do here, as the table is autogenertaed anyway, but indeed it is already there, explained, and even the script for generating it appears in the comments for reproducibility. > > I'm not sure I'd use shell for this, but since you already wrote it and > it works... > If you've noticed, the original script was in AWK. But to be as generic as possible, I didn't write the generation script in AWK because even AWK is not guaranteed to be installed on the build system. The only interpreted language that is guaranteed to be there is shell (most basic shell) because .configure itself needs it. Sure, the script could be prettier and shorter, but I wanted to keep it compatible with the most basic shell, not only Bash, and without needing external programs. Eric - thanks for the comment about 'local' - I will get rid of it if we decide to include this patch. > > Unchecked use of command-line arguments is not nice: > > $ scripts/gen-sizes.sh > scripts/gen-sizes.sh: line 64: : No such file or directory > scripts/gen-sizes.sh: line 65: : No such file or directory > scripts/gen-sizes.sh: line 66: : No such file or directory > > You should error out if $# -ne 1. But in such a simple script, I'd > dispense with arguments and print to stdout. Matter of taste. > Rejecting $# -ne 0 is still nice then. > Agree, thanks! Leonid. ___
