The sender domain has a DMARC Reject/Quarantine policy which disallows sending mailing list messages using the original "From" header.
To mitigate this problem, the original message has been wrapped automatically by the mailing list software.
--- Begin Message ---Thanks.Hi Ole Kristian, This patch has some formal issues. The patch title should start with "tplink-safeloader:";Sorry, bad mistake, hopefully won't ever happen again!see `git log src/tplink-safeloader.c` for examples. Furthermore, there are trailing white spaces, you've used DOS line endings (CRLF), and there's a number of other formatting issues. You can use openwrt.git:scripts/checkpatch.pl to detect these issues before submitting patches.Are you sure about this CRLF? I do all my development work in VSCode for Linux, the file is clearly marked as LF in VSCode, and cat -vTE doesn't show any ^Ms.Thanks for the checkoath.pl part. I didn't know about it, but will be using it from now on.On Fri, 2022-04-29 at 08:40 +0200, o...@oklona.net wrote:Some devices, specifically Deco M4R-v3 / M5. These devices have fallback partitions which will be used in case the device determines that the primary partition set is unbootable. It is believed that the backup partitions are populated by some mechanism in the OEM firmware, meaning a fallback tobackup-partitions will effectively revert the device back to OEM software.How these back-up partitions are created is not very relevant for this patch, so you canleave out the second paragraph. What is important to note however, is that thesepartitions do not use the standard list of partition names we've assumed until now. (The 'why is this patch needed' part of the commit message)Ok, noted.Signed-off-by: Ole Kristian Lona <o...@oklona.net> ---src/tplink-safeloader.c | 67 +++++++++++++++++++++++++++++++++--------1 file changed, 54 insertions(+), 13 deletions(-) diff --git a/src/tplink-safeloader.c b/src/tplink-safeloader.c index fc46124..ccb384f 100644 --- a/src/tplink-safeloader.c +++ b/src/tplink-safeloader.c @@ -53,6 +53,15 @@ struct flash_partition_entry { uint32_t size; }; +/** Flash partition names table entry */ +struct factory_partition_names { + const char *partition_table; + const char *soft_ver; + const char *os_image; + const char *file_system; + const char *extra_para; +};trailing whitespace+ /** Partition trailing padding definitions * Values 0x00 to 0xff are reserved to indicate the padding value * Values from 0x100 are reserved to indicate other behaviour */ @@ -89,6 +98,7 @@ struct device_info { struct flash_partition_entry partitions[MAX_PARTITIONS+1]; const char *first_sysupgrade_partition; const char *last_sysupgrade_partition; + struct factory_partition_names partition_names; }; #define SOFT_VER_TEXT(_t) {.type = SOFT_VER_TYPE_TEXT, .text = _t} @@ -2982,8 +2992,8 @@ static void set_source_date_epoch() { } /** Generates the partition-table partition */ -static struct image_partition_entry make_partition_table(const struct flash_partition_entry *p) {- struct image_partition_entry entry = alloc_image_partition("partition-table",0x800);+static struct image_partition_entry make_partition_table(const char *name, const structflash_partition_entry *p) {+ struct image_partition_entry entry = alloc_image_partition(name, 0x800);char *s = (char *)entry.data, *end = (char *)(s+entry.size); @@ -3018,14 +3028,14 @@ static inline uint8_t bcd(uint8_t v) { /** Generates the soft-version partition */ -static struct image_partition_entry make_soft_version(+static struct image_partition_entry make_soft_version(const char *name,const struct device_info *info, uint32_t rev) { /** If an info string is provided, use this instead of * the structured data, and include the null-termination */ if (info->soft_ver.type == SOFT_VER_TYPE_TEXT) { uint32_t len = strlen(info->soft_ver.text) + 1; - return init_meta_partition_entry("soft-version", + return init_meta_partition_entry(name, info->soft_ver.text, len, info->part_trail); }@@ -3055,11 +3065,11 @@ static struct image_partition_entry make_soft_version(}; if (info->soft_ver_compat_level == 0) - return init_meta_partition_entry("soft-version", &s, + return init_meta_partition_entry(name, &s,(uint8_t *)(&s.compat_level) - (uint8_t *)(&s),info->part_trail); else - return init_meta_partition_entry("soft-version", &s, + return init_meta_partition_entry(name, &s, sizeof(s), info->part_trail); } @@ -3298,7 +3308,7 @@ static void build_image(const char *output, bool sysupgrade, struct device_info *info) { - size_t i; + size_t i;spurious change due to added whitespacestruct image_partition_entry parts[7] = {}; @@ -3306,6 +3316,8 @@ static void build_image(const char *output, struct flash_partition_entry *os_image_partition = NULL; struct flash_partition_entry *file_system_partition = NULL; size_t firmware_partition_index = 0; + char fs_name[32]; + char os_name[32];These extra variables aren't required. You can just pass the const char * from the partition name table to the relevant functions. The device_info struct that is passed intothis function has a longer lifetime than (any data generated by) this function.Well, it depends on your preference on warnings. By not using these, you change pointers that are interpreted as const, and get warnings about that, which is the only reason I added these. I normally try as hard as I can to create code that does not create spurious warnings.for (i = 0; info->partitions[i].name; i++) { if (!strcmp(info->partitions[i].name, "firmware")) @@ -3330,7 +3342,13 @@ static void build_image(const char *output,for (i = MAX_PARTITIONS-1; i >= firmware_partition_index + 1; i--)info->partitions[i+1] = info->partitions[i]; - file_system_partition->name = "file-system"; + if(info->partition_names.file_system==NULL) + file_system_partition->name = "file-system"; + else{+ strcpy(fs_name, info->partition_names.file_system);+ file_system_partition->name = fs_name; + } +formatting issues: no spaces around '==', 'else{', trailing whitespace This can be simplified into: if (info->partition_names.file_system) file_system_partition->name = info->partition_names.file_system; else file_system_partition->name = "file-system";I was very much in doubt about this. Mostly, I wrote things in the simple way, but seem to have forgotten about this one. Most C programmers recommend using curly braces for readability, although I clearly prefer the simple notation.Same comments for the rest of the patch. Best, SanderI will send a new version of the path, with all of this fixed tomorrow. -But I am unsure on how to proceed with the fs_name[32] and os_name[32] variables, since removing them will make the code generate warnings.Thanks, Ole Kristian
--- End Message ---
_______________________________________________ openwrt-devel mailing list openwrt-devel@lists.openwrt.org https://lists.openwrt.org/mailman/listinfo/openwrt-devel
