| Message ID | 20240402123636.58237-1-newtwen+github@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | base-files: reduce IPv6 ULA prefix generation to a single call | expand |
On Tue, Apr 02, 2024 at 02:36:36PM +0200, Paul Donald wrote: > Tested on: 23.05.3 > > Signed-off-by: Paul Donald <newtwen+github@gmail.com> > --- > .../files/etc/uci-defaults/12_network-generate-ula | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/package/base-files/files/etc/uci-defaults/12_network-generate-ula b/package/base-files/files/etc/uci-defaults/12_network-generate-ula > index 19d7ed7f2e..20b3237ec7 100644 > --- a/package/base-files/files/etc/uci-defaults/12_network-generate-ula > +++ b/package/base-files/files/etc/uci-defaults/12_network-generate-ula > @@ -1,11 +1,9 @@ > [ "$(uci -q get network.globals.ula_prefix)" != "auto" ] && exit 0 > > -r1=$(dd if=/dev/urandom bs=1 count=1 |hexdump -e '1/1 "%02x"') > -r2=$(dd if=/dev/urandom bs=2 count=1 |hexdump -e '2/1 "%02x"') > -r3=$(dd if=/dev/urandom bs=2 count=1 |hexdump -e '2/1 "%02x"') > +r1=$(hexdump -vn 5 -e '5/1 "%02x"' /dev/urandom) > > uci -q batch <<-EOF >/dev/null > - set network.globals.ula_prefix=fd$r1:$r2:$r3::/48 > + set network.globals.ula_prefix=fd${r1:0:2}:${r1:2:4}:${r1:6:4}::/48 > commit network > EOF > > -- First, since you got rid of "r2" and "r3", "r1" now seems a bad name. I would suggest switching to simply "r". Second, appears the ${parameter:offset:length} may not be POSIX. I dislike this, but do not object since OpenWRT's shell is built with this functionality enabled. Third, you need a better commit message. Perhaps type something about how this improves things. Overall, I like the idea. This isn't a UUOC, but is pretty close. Cleanup is always valuable. Only #1 and #3 need to be addressed.
On 2024-04-02 23:00, Elliott Mitchell wrote: > Second, appears the ${parameter:offset:length} may not be POSIX. I > dislike this, but do not object since OpenWRT's shell is built with this > functionality enabled. UUOC! Ha. Yes, there are a few non POSIXy things in openwrt ash. A number of other scripts already take advantage of them so it's OK, if it avoids several external calls to e.g. cut or td. How about POSIX native array IFS split? IFS=' ' set -- $(hexdump -vn 5 -e '5/1 "%02x "' /dev/urandom) uci -q batch <<-EOF >/dev/null set network.globals.ula_prefix=fd$1:$2$3:$4$5::/48 commit network EOF
On Wed, Apr 03, 2024 at 12:50:50AM +0200, Paul D wrote: > On 2024-04-02 23:00, Elliott Mitchell wrote: > > Second, appears the ${parameter:offset:length} may not be POSIX. I > > dislike this, but do not object since OpenWRT's shell is built with this > > functionality enabled. > > > UUOC! Ha. Yes, there are a few non POSIXy things in openwrt ash. A number of other scripts already take advantage of them so it's OK, if it avoids several external calls to e.g. cut or td. > Yes, which is why even though I disliked it, I wouldn't be able to reject merely for that. > How about POSIX native array IFS split? > > > IFS=' ' set -- $(hexdump -vn 5 -e '5/1 "%02x "' /dev/urandom) > > uci -q batch <<-EOF >/dev/null > set network.globals.ula_prefix=fd$1:$2$3:$4$5::/48 > commit network > EOF That is certainly better than the solution I came up with. More importantly, it addresses concern #1. Now just need a better commit message and hopefully the committers would find it acceptable.
diff --git a/package/base-files/files/etc/uci-defaults/12_network-generate-ula b/package/base-files/files/etc/uci-defaults/12_network-generate-ula index 19d7ed7f2e..20b3237ec7 100644 --- a/package/base-files/files/etc/uci-defaults/12_network-generate-ula +++ b/package/base-files/files/etc/uci-defaults/12_network-generate-ula @@ -1,11 +1,9 @@ [ "$(uci -q get network.globals.ula_prefix)" != "auto" ] && exit 0 -r1=$(dd if=/dev/urandom bs=1 count=1 |hexdump -e '1/1 "%02x"') -r2=$(dd if=/dev/urandom bs=2 count=1 |hexdump -e '2/1 "%02x"') -r3=$(dd if=/dev/urandom bs=2 count=1 |hexdump -e '2/1 "%02x"') +r1=$(hexdump -vn 5 -e '5/1 "%02x"' /dev/urandom) uci -q batch <<-EOF >/dev/null - set network.globals.ula_prefix=fd$r1:$r2:$r3::/48 + set network.globals.ula_prefix=fd${r1:0:2}:${r1:2:4}:${r1:6:4}::/48 commit network EOF
Tested on: 23.05.3 Signed-off-by: Paul Donald <newtwen+github@gmail.com> --- .../files/etc/uci-defaults/12_network-generate-ula | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)