| Message ID | 20240328170108.13758-1-tmn505@terefe.re |
|---|---|
| State | Superseded |
| Headers | show |
| Series | scripts: gen_image_generic: allow FAT fs on kernel partition for non-GPT targets | expand |
Recommend avoiding -a and -o params. Use instead e.g. [ -n "$GUID" ] || [ "$KERNELPARTTYPE" = "6" ] || [ "$KERNELPARTTYPE" = "c" ] https://www.shellcheck.net/wiki/SC2166 On 2024-03-28 18:00, Tomasz Maciej Nowak wrote: > From: Tomasz Maciej Nowak <tmn505@gmail.com> > > Some old or proprietary bootloader recognize only FAT file system > variants on storage devices with MBR, unfortunately script ties > format of kernel partition to GPT partition table. So, allow kernel > partition file system to be FAT16 or FAT32 if appropriate type is set > in partition table. > > Signed-off-by: Tomasz Maciej Nowak <tmn505@gmail.com> > --- > scripts/gen_image_generic.sh | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/scripts/gen_image_generic.sh b/scripts/gen_image_generic.sh > index 11e40f38868f..44837fde1e12 100755 > --- a/scripts/gen_image_generic.sh > +++ b/scripts/gen_image_generic.sh > @@ -49,7 +49,7 @@ dos_dircopy() { > [ -n "$PADDING" ] && dd if=/dev/zero of="$OUTPUT" bs=512 seek="$ROOTFSOFFSET" conv=notrunc count="$ROOTFSSIZE" > dd if="$ROOTFSIMAGE" of="$OUTPUT" bs=512 seek="$ROOTFSOFFSET" conv=notrunc > > -if [ -n "$GUID" ]; then > +if [ -n "$GUID" -o "$KERNELPARTTYPE" = "6" -o "$KERNELPARTTYPE" = "c" ]; then > [ -n "$PADDING" ] && dd if=/dev/zero of="$OUTPUT" bs=512 seek="$((ROOTFSOFFSET + ROOTFSSIZE))" conv=notrunc count="$sect" > mkfs.fat --invariant -n kernel -C "$OUTPUT.kernel" -S 512 "$((KERNELSIZE / 1024))" > LC_ALL=C dos_dircopy "$KERNELDIR" /
On Fri, Mar 29, 2024 at 04:32:18PM +0100, Paul D wrote: > Recommend avoiding -a and -o params. > > Use instead e.g. > > [ -n "$GUID" ] || [ "$KERNELPARTTYPE" = "6" ] || [ "$KERNELPARTTYPE" = "c" ] > > https://www.shellcheck.net/wiki/SC2166 The examples pointed to amounted to "be careful with untrusted input to shell scripts". Build systems must already be pretty much 100% trusted. If someone slips something problematic into one there is pretty much nothing to be done about it. I've been getting the feeling the whole community is slowly trying to recreate SunOS 5.7^WSolaris 2.7^WSoliaris 7 (used to be a Sun Thiing, but now everyone's version numbers tend to be inflated). Where /bin/false, /bin/true and /bin/test were all Korne Shell scripts. This nominally saved development work since Korne shell has implementations of all these internally. Problem is this killed performance for any shell script /not/ written in a shell with those as built-in. While Korne shell is very fast once it has finished parsing its input, it is slow at parsing its scripts. Having `gunzip` be a shell script seems headed in this direction. The above seems a similar sort of situation, adding an extra fork()/execve() to avoid warnings. I'm placing SC2166 on my disregard list. Yes, this is nominally not well defined, but unlike most warnings this one has a major impact on performance. (fork()/execve() are the two most expensive routinely used system calls)
W dniu 29.03.2024 o 19:08, Elliott Mitchell pisze: > On Fri, Mar 29, 2024 at 04:32:18PM +0100, Paul D wrote: >> Recommend avoiding -a and -o params. >> >> Use instead e.g. >> >> [ -n "$GUID" ] || [ "$KERNELPARTTYPE" = "6" ] || [ "$KERNELPARTTYPE" = "c" ] >> >> https://www.shellcheck.net/wiki/SC2166 > > The examples pointed to amounted to "be careful with untrusted input to > shell scripts". Build systems must already be pretty much 100% trusted. > If someone slips something problematic into one there is pretty much > nothing to be done about it. > > > I've been getting the feeling the whole community is slowly trying to > recreate SunOS 5.7^WSolaris 2.7^WSoliaris 7 (used to be a Sun Thiing, > but now everyone's version numbers tend to be inflated). Where > /bin/false, /bin/true and /bin/test were all Korne Shell scripts. > > This nominally saved development work since Korne shell has > implementations of all these internally. Problem is this killed > performance for any shell script /not/ written in a shell with those as > built-in. While Korne shell is very fast once it has finished parsing > its input, it is slow at parsing its scripts. > > Having `gunzip` be a shell script seems headed in this direction. The > above seems a similar sort of situation, adding an extra fork()/execve() > to avoid warnings. > > I'm placing SC2166 on my disregard list. Yes, this is nominally not well > defined, but unlike most warnings this one has a major impact on > performance. > > (fork()/execve() are the two most expensive routinely used system calls) > Thanks, I did consider the execution penalty but as this script is small I decided to go with Paul's suggestion, and as bonus, just to avoid future noise around this, when someone else validates this script with shellcheck.
diff --git a/scripts/gen_image_generic.sh b/scripts/gen_image_generic.sh index 11e40f38868f..44837fde1e12 100755 --- a/scripts/gen_image_generic.sh +++ b/scripts/gen_image_generic.sh @@ -49,7 +49,7 @@ dos_dircopy() { [ -n "$PADDING" ] && dd if=/dev/zero of="$OUTPUT" bs=512 seek="$ROOTFSOFFSET" conv=notrunc count="$ROOTFSSIZE" dd if="$ROOTFSIMAGE" of="$OUTPUT" bs=512 seek="$ROOTFSOFFSET" conv=notrunc -if [ -n "$GUID" ]; then +if [ -n "$GUID" -o "$KERNELPARTTYPE" = "6" -o "$KERNELPARTTYPE" = "c" ]; then [ -n "$PADDING" ] && dd if=/dev/zero of="$OUTPUT" bs=512 seek="$((ROOTFSOFFSET + ROOTFSSIZE))" conv=notrunc count="$sect" mkfs.fat --invariant -n kernel -C "$OUTPUT.kernel" -S 512 "$((KERNELSIZE / 1024))" LC_ALL=C dos_dircopy "$KERNELDIR" /