Re: [PATCH V2 1/3] base-files: sysupgrade: add tar.sh with helpers for building archives

[Jo-Philipp Wich]

Hi Rafał,

comments inline. Sorry for the bikeshedding ahead.

~ Jo


[...]
+
+__tar_print_padding() {
+   dd if=/dev/zero bs=$1 count=1 2>/dev/null


$1 may be 0 which is an invalid value for `bs=`:

  root@OpenWrt:~# dd bs=0
  dd: number 0 is not in 1..2147483647 range

A value of "0" is valid for `count=` though:

  root@OpenWrt:~# dd bs=1 count=0
  0+0 records in
  0+0 records out

So either revert to the previous version or hardcode `bs` to 1, pass the
desired size via `count=` and live with slightly less efficiency (which likely
does not matter at all).


+}
+
+__tar_make_member() {
+   local name="$1"
+   local content="$2"


If you make this `local mtime=${3:-$(date +%s)}` you can rename this function 
to `tar_make_member_inline()` and drop the extra wrapper.



+   local mtime="$3"
+   local mode=644
[...]
+}
+
+tar_make_member_from_file() {


This `tar_make_member_from_file()` wrapper is unused and very trivial, the
only thing it adds is defaulting the mtime value to the mtime of the given
file path. I suggest to drop `tar_make_member_from_file()` entirely and to
rename  `tar_make_member_inline()` below to simply `tar_make_member()`.

Maybe also consider renaming it to `tar_print_member()` or
`tar_output_member()` since that is what it does.


+   local name="$1"
+
+   __tar_make_member "$name" "$(cat $name)" "$(date +%s -r "$1")"
+}
+
+tar_make_member_inline() {
+   local name="$1"
+   local content="$2"
+   local mtime="${3:-$(date +%s)}"
+
+   __tar_make_member "$name" "$content" "$mtime"
+}
+


Analogous to the suggested name change above, the `tar_close()` function 
should probably be renamed to `tar_print_trailer()` or `tar_output_trailer()`

as well.


+tar_close() {
+   __tar_print_padding 1024
+}


___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

[PATCH V2 1/3] base-files: sysupgrade: add tar.sh with helpers for building archives

[Rafał Miłecki]

From: Jo-Philipp Wich 

This allows building uncompressed tar archives from shell scripts (and
compressing them later if needed)

Signed-off-by: Rafał Miłecki 
---
V2: Simplify dd in __tar_print_padding (I still think helper is useful)
Hardcode 0/0/ root/root for now as most likely it'll be enough
Simplify name validation (leasing slash)
Reorder some variables

 package/base-files/files/lib/upgrade/tar.sh | 78 +
 1 file changed, 78 insertions(+)
 create mode 100644 package/base-files/files/lib/upgrade/tar.sh

diff --git a/package/base-files/files/lib/upgrade/tar.sh 
b/package/base-files/files/lib/upgrade/tar.sh
new file mode 100644
index 00..406d5fd71b
--- /dev/null
+++ b/package/base-files/files/lib/upgrade/tar.sh
@@ -0,0 +1,78 @@
+# SPDX-License-Identifier: GPL-2.0-or-later OR MIT
+
+__tar_print_padding() {
+   dd if=/dev/zero bs=$1 count=1 2>/dev/null
+}
+
+__tar_make_member() {
+   local name="$1"
+   local content="$2"
+   local mtime="$3"
+   local mode=644
+   local uid=0
+   local gid=0
+   local size=${#content}
+   local type=0
+   local link=""
+   local username="root"
+   local groupname="root"
+
+   # 100 byte of padding bytes, using 0x01 since the shell does not 
tolerate null bytes in strings
+   local 
pad=$'\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1'
+
+   # validate name (strip leading slash if present)
+   name=${name#/}
+
+   # truncate string header values to their maximum length
+   name=${name:0:100}
+   link=${link:0:100}
+   username=${username:0:32}
+   groupname=${groupname:0:32}
+
+   # construct header part before checksum field
+   local header1="${name}${pad:0:$((100 - ${#name}))}"
+   header1="${header1}$(printf '%07d\1' $mode)"
+   header1="${header1}$(printf '%07o\1' $uid)"
+   header1="${header1}$(printf '%07o\1' $gid)"
+   header1="${header1}$(printf '%011o\1' $size)"
+   header1="${header1}$(printf '%011o\1' $mtime)"
+
+   # construct header part after checksum field
+   local header2="$(printf '%d' $type)"
+   header2="${header2}${link}${pad:0:$((100 - ${#link}))}"
+   header2="${header2}ustar  ${pad:0:1}"
+   header2="${header2}${username}${pad:0:$((32 - ${#username}))}"
+   header2="${header2}${groupname}${pad:0:$((32 - ${#groupname}))}"
+
+   # calculate checksum over header fields
+   local checksum=0
+   for byte in $(printf '%s%8s%s' "$header1" "" "$header2" | tr '\1' '\0' 
| hexdump -ve '1/1 "%u "'); do
+   checksum=$((checksum + byte))
+   done
+
+   # print member header, padded to 512 byte
+   printf '%s%06o\0 %s' "$header1" $checksum "$header2" | tr '\1' '\0'
+   __tar_print_padding 183
+
+   # print content data, padded to multiple of 512 byte
+   printf "%s" "$content"
+   __tar_print_padding $((512 - (size % 512)))
+}
+
+tar_make_member_from_file() {
+   local name="$1"
+
+   __tar_make_member "$name" "$(cat $name)" "$(date +%s -r "$1")"
+}
+
+tar_make_member_inline() {
+   local name="$1"
+   local content="$2"
+   local mtime="${3:-$(date +%s)}"
+
+   __tar_make_member "$name" "$content" "$mtime"
+}
+
+tar_close() {
+   __tar_print_padding 1024
+}
-- 
2.35.3


___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel