[PATCH] tplink-safeloader: Patch to handle partitions with alternate names.

[Rafał Miłecki]

On 2.05.2022 18:24, Ole Kristian Lona via openwrt-devel wrote:
> 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.

Please kindly fix your domain setup. That ML workaround with forwarding
makes it hard to read / review / apply your changes.

 > @@ -3306,6 +3318,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];
 >
 >  	for (i = 0; info->partitions[i].name; i++) {
 >  		if (!strcmp(info->partitions[i].name, "firmware"))

I don't think you need those fs_name / os_name. Just add "const" where
it's needed.


 > @@ -3339,15 +3359,38 @@  static void build_image(const char *output,
 >
 >  		file_system_partition->size = firmware_partition->size - file_system_partition->base;
 >
 > -		os_image_partition->name = "os-image";
 > +		if (info->partition_names.os_image == NULL)
 > +			os_image_partition->name = "os-image";
 > +		else {
 > +			strcpy(os_name, info->partition_names.os_image);
 > +			os_image_partition->name = os_name;
 > +		}
 > +
 >  		os_image_partition->size = kernel.st_size;
 >  	}
 >
 > -	parts[0] = make_partition_table(info->partitions);
 > -	parts[1] = make_soft_version(info, rev);
 > +	if (info->partition_names.partition_table == NULL)
 > +		parts[0] = make_partition_table("partition-table", info->partitions);
 > +	else
 > +		parts[0] = make_partition_table(info->partition_names.partition_table, info->partitions);
 > +
 > +	if (info->partition_names.soft_ver == NULL)
 > +		parts[1] = make_soft_version("soft-version", info, rev);
 > +	else
 > +		parts[1] = make_soft_version(info->partition_names.soft_ver, info, rev);
 > +
 >  	parts[2] = make_support_list(info);
 > -	parts[3] = read_file("os-image", kernel_image, false, NULL);
 > -	parts[4] = read_file("file-system", rootfs_image, add_jffs2_eof, file_system_partition);
 > +
 > +	if (info->partition_names.os_image == NULL)
 > +		parts[3] = read_file("os-image", kernel_image, false, NULL);
 > +	else
 > +		parts[3] = read_file(info->partition_names.os_image, kernel_image, false, NULL);
 > +
 > +	if (info->partition_names.file_system == NULL)
 > +		parts[4] = read_file("file-system", rootfs_image, add_jffs2_eof, file_system_partition);
 > +	else
 > +		parts[4] = read_file(info->partition_names.file_system, rootfs_image, add_jffs2_eof, file_system_partition);
 > +
 >
 >  	/* Some devices need the extra-para partition to accept the firmware */
 >  	if (strcasecmp(info->id, "ARCHER-A6-V3") == 0 ||

We now have all those (foo == NULL) checks everywhere. We also have
default names inlined all around.

What about adding new function that will setup default names if custom
ones are not specified? That would gather all checking code in one place
and would let you move all NULL checks to a single place.

set_default_names()
{
	if (!info->partition_names.partition_table)
		info->partition_names.partition_table = "partition-table";
	if (!info->partition_names.soft_ver)
		info->partition_names.soft_ver = "soft-version";
	(...)
}