Hello Karl, On 8/16/18 3:12 AM, Karl Palsson wrote: > 1) how bad are the portability issues that you felt > reimplementing in _C_ was the best path? 2) if it's that bad, why > keep it? How will future people knwo what to use. Either get rid > of it, or fix it.
Regarding 1) The portability issue seems to be affecting older bash versions. Christian described and also fixed the issue in the forums [1]. I'm not sure about what systems correctly are affected (definitely older MacOS X and the OS of the buildbots). The C implementation originated from when i worked on the device before finding Christians tree. Honestly i also think the readability of the current script is not really that great but honestly i don't think we can substantially improve that. Regarding 2) I see that point, removing it is probably the better way. [1] https://forum.openwrt.org/t/solved-zyxel-nbg6617-tftp-flash-wont-take/17731/10 Greetings David > Cheers, > Karl P > >> >> Signed-off-by: David Bauer <m...@david-bauer.net> >> --- >> include/image-commands.mk | 13 + >> target/linux/ipq40xx/image/Makefile | 2 +- >> tools/firmware-utils/Makefile | 1 + >> tools/firmware-utils/src/mkrasimage.c | 374 ++++++++++++++++++++++++++ >> 4 files changed, 389 insertions(+), 1 deletion(-) >> create mode 100644 tools/firmware-utils/src/mkrasimage.c >> >> diff --git a/include/image-commands.mk >> b/include/image-commands.mk index 3cc5dc21e1..bb5fe46e1a 100644 >> --- a/include/image-commands.mk >> +++ b/include/image-commands.mk >> @@ -62,6 +62,19 @@ define Build/make-ras >> @mv $@.new $@ >> endef >> >> +define Build/zyxel-ras-image >> + let \ >> + newsize="$(subst k,* 1024,$(RAS_ROOTFS_SIZE))"; \ >> + $(STAGING_DIR_HOST)/bin/mkrasimage \ >> + -b $(RAS_BOARD) \ >> + -v $(RAS_VERSION) \ >> + -k $(call >> param_get_default,kernel,$(1),$(IMAGE_KERNEL)) \ >> + -r $@ \ >> + -s $$newsize \ >> + -o $@.new >> + @mv $@.new $@ >> +endef >> + >> define Build/mkbuffaloimg >> $(STAGING_DIR_HOST)/bin/mkbuffaloimg -B $(BOARDNAME) \ >> -R $$(($(subst k, * 1024,$(ROOTFS_SIZE)))) \ >> diff --git a/target/linux/ipq40xx/image/Makefile >> b/target/linux/ipq40xx/image/Makefile index >> d1ee1004fd..6e4125db0b 100644 >> --- a/target/linux/ipq40xx/image/Makefile >> +++ b/target/linux/ipq40xx/image/Makefile >> @@ -221,7 +221,7 @@ define Device/zyxel_nbg6617 >> # at least as large as the one of the initial firmware image (not the >> current >> # one on the device). This only applies to the Web-UI, the bootlaoder >> ignores >> # this minimum-size. However, the larger image can be flashed both ways. >> - IMAGE/factory.bin := append-rootfs | pad-rootfs | check-size >> $$$$(ROOTFS_SIZE) | make-ras >> + IMAGE/factory.bin := append-rootfs | pad-rootfs | check-size >> $$$$(ROOTFS_SIZE) | zyxel-ras-image >> IMAGE/sysupgrade.bin/squashfs := append-rootfs | pad-rootfs | >> check-size $$$$(ROOTFS_SIZE) | sysupgrade-tar rootfs=$$$$@ | append-metadata >> DEVICE_PACKAGES := ipq-wifi-zyxel_nbg6617 uboot-envtools >> endef >> diff --git a/tools/firmware-utils/Makefile >> b/tools/firmware-utils/Makefile index 436a43794c..00917c3417 >> 100644 >> --- a/tools/firmware-utils/Makefile >> +++ b/tools/firmware-utils/Makefile >> @@ -70,6 +70,7 @@ define Host/Compile >> $(call cc,fix-u-media-header cyg_crc32,-Wall) >> $(call cc,hcsmakeimage bcmalgo) >> $(call cc,mkporayfw, -Wall) >> + $(call cc,mkrasimage, --std=gnu99) >> $(call cc,mkhilinkfw, -lcrypto) >> $(call cc,mkdcs932, -Wall) >> $(call cc,mkheader_gemtek,-lz) >> diff --git a/tools/firmware-utils/src/mkrasimage.c >> b/tools/firmware-utils/src/mkrasimage.c new file mode 100644 >> index 0000000000..1cac7b5da9 >> --- /dev/null >> +++ b/tools/firmware-utils/src/mkrasimage.c >> @@ -0,0 +1,374 @@ >> +/* >> + * --- ZyXEL header format --- >> + * Original Version by Benjamin Berg <benja...@sipsolutions.net> >> + * C implementation based on generation-script by Christian Lamparter >> <chunk...@gmail.com> >> + * >> + * The firmware image prefixed with a header (which is written into the MTD >> device). >> + * The header is one erase block (~64KiB) in size, but the checksum only >> convers the >> + * first 2KiB. Padding is 0xff. All integers are in big-endian. >> + * >> + * The checksum is always a 16-Bit System V checksum (sum -s) stored in a >> 32-Bit integer. >> + * >> + * 4 bytes: checksum of the rootfs image >> + * 4 bytes: length of the contained rootfs image file (big endian) >> + * 32 bytes: Firmware Version string (NUL terminated, 0xff padded) >> + * 4 bytes: checksum over the header partition (big endian - see below) >> + * 64 bytes: Model (e.g. "NBG6617", NUL termiated, 0xff padded) >> + * 4 bytes: checksum of the kernel partition >> + * 4 bytes: length of the contained kernel image file (big endian) >> + * rest: 0xff padding (To erase block size) >> + * >> + * The checksums are calculated by adding up all bytes and if a 16bit >> + * overflow occurs, one is added and the sum is masked to 16 bit: >> + * csum = csum + databyte; if (csum > 0xffff) { csum += 1; csum &= 0xffff >> }; >> + * Should the file have an odd number of bytes then the byte len-0x800 is >> + * used additionally. >> + * >> + * The checksum for the header is calculated over the first 2048 bytes with >> + * the rootfs image checksum as the placeholder during calculation. >> + * >> + * This program is free software; you can redistribute it and/or modify it >> + * under the terms of the GNU General Public License version 2 as published >> + * by the Free Software Foundation. >> + * >> + */ >> +#include <byteswap.h> >> +#include <getopt.h> >> +#include <libgen.h> >> +#include <stdio.h> >> +#include <string.h> >> +#include <stdlib.h> >> + >> +#define VERSION_STRING_LEN 31 >> +#define ROOTFS_HEADER_LEN 40 >> + >> +#define KERNEL_HEADER_LEN 8 >> + >> +#define BOARD_NAME_LEN 64 >> +#define BOARD_HEADER_LEN 68 >> + >> +#define HEADER_PARTITION_CALC_LENGTH 2048 >> +#define HEADER_PARTITION_LENGTH 0x10000 >> + >> +struct file_info { >> + char *name; /* name of the file */ >> + char *data; /* file content */ >> + size_t size; /* length of the file */ >> +}; >> + >> +static char *progname; >> + >> +static char *board_name = 0; >> +static char *version_name = 0; >> +static unsigned int rootfs_size = 0; >> + >> +static struct file_info kernel = {0, 0, 0}; >> +static struct file_info rootfs = {0, 0, 0}; >> +static struct file_info rootfs_out = {0, 0, 0}; >> +static struct file_info out = {0, 0, 0}; >> + >> +#define ERR(fmt, ...) do { \ >> + fflush(0); \ >> + fprintf(stderr, "[%s] *** error: " fmt "\n", \ >> + progname, ## __VA_ARGS__ ); \ >> +} while (0) >> + >> +void bufferFile(struct file_info *finfo) { >> + unsigned int fs = 0; >> + FILE *f = NULL; >> + >> + f = fopen(finfo->name, "rb"); >> + if (f == NULL) { >> + printf("Error while opening file %s.", finfo->name); >> + exit(EXIT_FAILURE); >> + } >> + >> + fseek(f, 0L, SEEK_END); >> + fs = (unsigned int) ftell(f); >> + rewind(f); >> + >> + finfo->size = fs; >> + >> + char *data = malloc(fs); >> + finfo->data = data; >> + size_t read = fread(data, fs, 1, f); >> + >> + if (read != 1) { >> + printf("Error reading file %s.", finfo->name); >> + exit(EXIT_FAILURE); >> + } >> + >> + fclose(f); >> +} >> + >> +void writeFile(struct file_info *finfo) { >> + FILE *fout = fopen(finfo->name, "w"); >> + >> + if (!fwrite(finfo->data, finfo->size, 1, fout)) { >> + printf("Wanted to write, but something went wrong.\n"); >> + exit(1); >> + } >> +} >> + >> +void usage(int status) { >> + FILE *stream = (status != EXIT_SUCCESS) ? stderr : stdout; >> + >> + fprintf(stream, "Usage: %s [OPTIONS...]\n", progname); >> + fprintf(stream, >> + "\n" >> + "Options:\n" >> + " -k <kernel> path for kernel image\n" >> + " -r <rootfs> path for rootfs image\n" >> + " -s <rfssize> size of output rootfs\n" >> + " -v <version> version string\n" >> + " -b <boardname> name of board to generate image for\n" >> + " -h show this screen\n" >> + ); >> + >> + exit(status); >> +} >> + >> +static int sysv_chksm(const unsigned char *data, int size) { >> + int r; >> + int checksum; >> + unsigned int s = 0; /* The sum of all the input bytes, modulo (UINT_MAX >> + 1). */ >> + >> + >> + for (int i = 0; i < size; i++) { >> + s += data[i]; >> + } >> + >> + r = (s & 0xffff) + ((s & 0xffffffff) >> 16); >> + checksum = (r & 0xffff) + (r >> 16); >> + >> + return checksum; >> +} >> + >> +char *generate_rootfs_header(struct file_info rootfs, char >> *version) { >> + size_t version_string_length; >> + unsigned int chksm, size; >> + char *rootfs_header; >> + size_t ptr = 0; >> + >> + rootfs_header = malloc(ROOTFS_HEADER_LEN); >> + memset(rootfs_header, 0xff, ROOTFS_HEADER_LEN); >> + >> + memcpy(rootfs_out.data, rootfs.data, rootfs.size); >> + >> + chksm = __bswap_32(sysv_chksm(rootfs_out.data, rootfs_out.size)); >> + size = __bswap_32(rootfs_out.size); >> + >> + memcpy(rootfs_header + ptr, &chksm, 4); >> + ptr += 4; >> + >> + memcpy(rootfs_header + ptr, &size, 4); >> + ptr += 4; >> + >> + version_string_length = strlen(version) <= VERSION_STRING_LEN ? >> strlen(version) : VERSION_STRING_LEN; >> + >> + memcpy(rootfs_header + ptr, version, version_string_length); >> + ptr += version_string_length; >> + >> + rootfs_header[ptr] = 0x0; >> + >> + return rootfs_header; >> +} >> + >> +char *generate_kernel_header(struct file_info kernel) { >> + unsigned int chksm, size; >> + char *kernel_header; >> + size_t ptr = 0; >> + >> + kernel_header = malloc(KERNEL_HEADER_LEN); >> + memset(kernel_header, 0xff, KERNEL_HEADER_LEN); >> + >> + chksm = __bswap_32(sysv_chksm(kernel.data, kernel.size)); >> + size = __bswap_32(kernel.size); >> + >> + memcpy(kernel_header + ptr, &chksm, 4); >> + ptr += 4; >> + >> + memcpy(kernel_header + ptr, &size, 4); >> + >> + return kernel_header; >> +} >> + >> +unsigned int generate_board_header_checksum(char *kernel_hdr, >> char *rootfs_hdr, char *boardname) { >> + char *board_hdr_tmp; >> + size_t ptr = 0; >> + >> + board_hdr_tmp = malloc(HEADER_PARTITION_CALC_LENGTH); >> + memset(board_hdr_tmp, 0xff, HEADER_PARTITION_CALC_LENGTH); >> + >> + memcpy(board_hdr_tmp, rootfs_hdr, ROOTFS_HEADER_LEN); >> + ptr += ROOTFS_HEADER_LEN; >> + >> + memcpy(board_hdr_tmp + ptr, rootfs_hdr, 4); /* Use RootFS checksum as >> placeholder */ >> + ptr += 4; >> + >> + memcpy(board_hdr_tmp + ptr, boardname, strlen(boardname)); /* Append >> boardname */ >> + ptr += strlen(boardname); >> + >> + board_hdr_tmp[ptr] = 0x0; /* Add null-terminator */ >> + ptr = ROOTFS_HEADER_LEN + 4 + BOARD_NAME_LEN; >> + >> + memcpy(board_hdr_tmp + ptr, kernel_hdr, 8); >> + >> + return __bswap_32(sysv_chksm(board_hdr_tmp, 2048)); >> +} >> + >> +char *generate_board_header(char *kernel_hdr, char >> *rootfs_hdr, char *boardname) { >> + unsigned int board_checksum; >> + char *board_hdr; >> + >> + board_hdr = malloc(BOARD_HEADER_LEN); >> + memset(board_hdr, 0xff, BOARD_HEADER_LEN); >> + >> + board_checksum = generate_board_header_checksum(kernel_hdr, rootfs_hdr, >> boardname); >> + >> + memcpy(board_hdr, &board_checksum, 4); >> + memcpy(board_hdr + 4, boardname, strlen(boardname)); >> + board_hdr[4 + strlen(boardname)] = 0x0; >> + >> + return board_hdr; >> +} >> + >> +int build_image() { >> + char *rootfs_header, *kernel_header, *board_header; >> + >> + size_t ptr; >> + >> + /* Load files */ >> + bufferFile(&kernel); >> + bufferFile(&rootfs); >> + >> + /* Allocate memory for temporary ouput rootfs */ >> + rootfs_out.data = malloc(rootfs_out.size); >> + memset(rootfs_out.data, 0x00, rootfs_out.size); >> + >> + /* Prepare headers */ >> + rootfs_header = generate_rootfs_header(rootfs, version_name); >> + kernel_header = generate_kernel_header(kernel); >> + board_header = generate_board_header(kernel_header, rootfs_header, >> board_name); >> + >> + /* Prepare output file */ >> + out.size = HEADER_PARTITION_LENGTH + rootfs_out.size + kernel.size; >> + out.data = malloc(out.size); >> + memset(out.data, 0xFF, out.size); >> + >> + /* Build output image */ >> + memcpy(out.data, rootfs_header, ROOTFS_HEADER_LEN); >> + memcpy(out.data + ROOTFS_HEADER_LEN, board_header, BOARD_HEADER_LEN); >> + memcpy(out.data + ROOTFS_HEADER_LEN + BOARD_HEADER_LEN, kernel_header, >> KERNEL_HEADER_LEN); >> + ptr = HEADER_PARTITION_LENGTH; >> + memcpy(out.data + ptr, rootfs_out.data, rootfs_out.size); >> + ptr += rootfs_out.size; >> + memcpy(out.data + ptr, kernel.data, kernel.size); >> + >> + /* Write back output image */ >> + writeFile(&out); >> + >> + /* Free allocated memory */ >> + free(kernel.data); >> + free(rootfs.data); >> + free(out.data); >> + free(rootfs_out.data); >> + >> + free(rootfs_header); >> + free(kernel_header); >> + free(board_header); >> + >> + return 0; >> +} >> + >> +int check_options() { >> + if (kernel.name == 0) { >> + ERR("No kernel filename supplied"); >> + return -1; >> + } >> + >> + if (rootfs.name == 0) { >> + ERR("No rootfs filename supplied"); >> + return -2; >> + } >> + >> + if (out.name == 0) { >> + ERR("No output filename supplied"); >> + return -3; >> + } >> + >> + if (board_name == 0) { >> + ERR("No board-name supplied"); >> + return -4; >> + } >> + >> + if (version_name == 0) { >> + ERR("No version supplied"); >> + return -5; >> + } >> + >> + if (rootfs_size <= 0) { >> + ERR("Invalid rootfs size supplied"); >> + return -6; >> + } >> + >> + if (strlen(board_name) > 31) { >> + ERR("Board name is to long"); >> + return -7; >> + } >> + return 0; >> +} >> + >> +int main(int argc, char *argv[]) { >> + int ret; >> + progname = basename(argv[0]); >> + while (1) { >> + int c; >> + >> + c = getopt(argc, argv, "b:k:o:r:s:v:h"); >> + if (c == -1) >> + break; >> + >> + switch (c) { >> + case 'b': >> + board_name = optarg; >> + break; >> + case 'h': >> + usage(EXIT_SUCCESS); >> + break; >> + case 'k': >> + kernel.name = optarg; >> + break; >> + case 'o': >> + out.name = optarg; >> + break; >> + case 'r': >> + rootfs.name = optarg; >> + break; >> + case 's': >> + sscanf(optarg, "%u", &rootfs_size); >> + break; >> + case 'v': >> + version_name = optarg; >> + break; >> + default: >> + usage(EXIT_FAILURE); >> + break; >> + } >> + } >> + >> + ret = check_options(); >> + if (ret) >> + usage(EXIT_FAILURE); >> + >> + /* As ZyXEL Web-GUI only accept images with a rootfs equal or larger >> than the first firmware shipped >> + * for the device, we need to pad rootfs partition to this size. To >> perform further calculations, we >> + * decide the size of this part here. In case the rootfs we want to >> integrate in our image is larger, >> + * take it's size, otherwise the supplied size. >> + * >> + * Be careful! We rely on assertion of correct size to be performed >> beforehand. It is unknown if images >> + * with a to large rootfs are accepted or not. >> + */ >> + rootfs_out.size = rootfs_size < rootfs.size ? rootfs.size : rootfs_size; >> + return build_image(); >> +} >> >> _______________________________________________ >> openwrt-devel mailing list >> openwrt-devel@lists.openwrt.org >> https://lists.openwrt.org/mailman/listinfo/openwrt-devel _______________________________________________ openwrt-devel mailing list openwrt-devel@lists.openwrt.org https://lists.openwrt.org/mailman/listinfo/openwrt-devel
