| Message ID | 20210104111054.1417-1-freifunk@adrianschmutzler.de |
|---|---|
| State | Rejected |
| Delegated to: | Adrian Schmutzler |
| Headers | show |
| Series | base-files: read all 5 bytes in get_magic_fat32() at once | expand |
The fact that you had to introduce iflags to work around changing IBS/OBS which afffected skip size would make me consider this sort of patch to be of incredibly marginal utilty, largely even negative value if this isn't some sort of hot path. You've made it more complicated for an _extremely_ small, un measured pseudo gain. I'd kinda imagine that the underlying layer already read a bigger block anyway... Sincerely, Karl Palsson Adrian Schmutzler <freifunk@adrianschmutzler.de> wrote: > While the speed improvement might be negligible, there is still > no reason to read individual bytes. > > Suggested-by: Paul Spooren <mail@aparcar.org> > Signed-off-by: Adrian Schmutzler <freifunk@adrianschmutzler.de> ----- > > get_magic_fat32() { > - (get_image "$@" | dd bs=1 count=5 skip=82) 2>/dev/null > + (get_image "$@" | dd iflag=skip_bytes bs=5 count=1 skip=82) 2>/dev/null > } >
> -----Original Message----- > From: openwrt-devel [mailto:openwrt-devel-bounces@lists.openwrt.org] > On Behalf Of Karl Palsson > Sent: Montag, 4. Januar 2021 12:58 > To: Adrian Schmutzler <freifunk@adrianschmutzler.de> > Cc: openwrt-devel <openwrt-devel@lists.openwrt.org> > Subject: Re: [PATCH] base-files: read all 5 bytes in get_magic_fat32() at once > > The fact that you had to introduce iflags to work around changing IBS/OBS > which afffected skip size would make me consider this sort of patch to be of > incredibly marginal utilty, largely even negative value if this isn't some sort of > hot path. You've made it more complicated for an _extremely_ small, un > measured pseudo gain. Indeed, that's why I initially only touched vfat, until Paul "requested" to update this one as well. I do not care terribly about this, I just noticed it when looking at the relevant section. I'd merge the "first" patch for vfat, as there is really no drawback there, but I don't really care what happens to this one. Best Adrian > > I'd kinda imagine that the underlying layer already read a bigger block > anyway... > > Sincerely, > Karl Palsson > > > Adrian Schmutzler <freifunk@adrianschmutzler.de> wrote: > > While the speed improvement might be negligible, there is still no > > reason to read individual bytes. > > > > Suggested-by: Paul Spooren <mail@aparcar.org> > > Signed-off-by: Adrian Schmutzler <freifunk@adrianschmutzler.de> > ----- > > > > get_magic_fat32() { > > - (get_image "$@" | dd bs=1 count=5 skip=82) 2>/dev/null > > + (get_image "$@" | dd iflag=skip_bytes bs=5 count=1 skip=82) > > +2>/dev/null > > } > >
diff --git a/package/base-files/Makefile b/package/base-files/Makefile index 8d40eb0e49..f18f221129 100644 --- a/package/base-files/Makefile +++ b/package/base-files/Makefile @@ -12,7 +12,7 @@ include $(INCLUDE_DIR)/version.mk include $(INCLUDE_DIR)/feeds.mk PKG_NAME:=base-files -PKG_RELEASE:=245 +PKG_RELEASE:=246 PKG_FLAGS:=nonshared PKG_FILE_DEPENDS:=$(PLATFORM_DIR)/ $(GENERIC_PLATFORM_DIR)/base-files/ diff --git a/package/base-files/files/lib/upgrade/common.sh b/package/base-files/files/lib/upgrade/common.sh index c28bae48a1..9b17e2ac5d 100644 --- a/package/base-files/files/lib/upgrade/common.sh +++ b/package/base-files/files/lib/upgrade/common.sh @@ -134,7 +134,7 @@ get_magic_vfat() { } get_magic_fat32() { - (get_image "$@" | dd bs=1 count=5 skip=82) 2>/dev/null + (get_image "$@" | dd iflag=skip_bytes bs=5 count=1 skip=82) 2>/dev/null } part_magic_efi() {
While the speed improvement might be negligible, there is still no reason to read individual bytes. Suggested-by: Paul Spooren <mail@aparcar.org> Signed-off-by: Adrian Schmutzler <freifunk@adrianschmutzler.de> --- package/base-files/Makefile | 2 +- package/base-files/files/lib/upgrade/common.sh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)