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

[Ole Kristian Lona]

Hmm. I get what you are saying. Obviously, I never added "support-list" to the factory_partition_names struct, since I believe we will never use it. But by adding it, we would hopefully get rid of all hard-coded partition names outside of device definitions, which would also clean up the "old" code at the same time.

-But I suppose you are still happy with a "factory_partition_names" struct within the device_info, since it is almost impossible, and may be unreliable to try to deduce the names from the partition scheme? I can make that modification immediately.

At the same time, I get warnings about long lines in the patch, but it looks like there are many long lines in the file already. Do we shorten the ones in the patch, or should I just leave them as is?


Regards,

Ole Kristian

On 12/05/2022, 16:24, "Sander Vanheule" <sander at svanheule.net> wrote:

    Hi Ole Kristian,

    On Thu, 2022-05-12 at 14:47 +0200, Ole Kristian Lona wrote:
    > > >                         info->soft_ver.text, len, info->part_trail);
    > > >         }
    > > >  
    > > > @@ -3055,11 +3082,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);
    > > >  }
    > > >  
    > > 
    > > Shouldn't you also modify make_support_list() and make_extra_para()?
    > 
    > I believe "support-list" will never change the name, and it definitely hasn't
    > in the firmwares I have seen. It sounds more logical to me that they always
    > keep that name, so any device can check the support list for any firmware.
    > 
    > Extra-para is only used for a very few devices, but I now added an option for
    > using a different name for that as well.

    Even if they aren't modified in this particular case, I was mainly worried about
    consistency. The partition names are now all stored in the device_info struct,
    so IMHO it would be cleaner to pull them all the info from there. Even if the
    default is never changed. Otherwise it's a bit pointless to  store the partition
    names in a modifiable container anyway.

    Best,
    Sander