Re: [PATCH 1/2] Add support for extlinux.conf

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

Hello,

On 4/24/26 10:46 AM, Alexander Shiyan wrote:
>> On 4/16/26 3:45 PM, Alexander Shiyan wrote:
>>> This adds support for the extlinux.conf configuration format, commonly
>>> used by Syslinux and many Linux distributions. The configuration file
>>> is typically located at /boot/extlinux/extlinux.conf or
>>> /extlinux/extlinux.conf and defines boot entries with kernel, initrd,
>>> device tree, and command line options.
> ...
>>> +static int extlinux_boot(struct bootentry *be, int verbose, int dryrun)
>>> +{
>>> +     struct extlinux_entry *e =
>>> +             container_of(be, struct extlinux_entry, entry);
>>> +     char *kernel_abs, *initrd_abs = NULL, *fdt_abs = NULL;
>>> +     struct bootm_data data = {};
>>> +     int ret;
>>> +
>>> +     bootm_data_init_defaults(&data);
>>> +
>>> +     data.dryrun = max_t(int, dryrun, data.dryrun);
>>> +     data.verbose = max(verbose, data.verbose);
>>> +     data.appendroot = true;
>>
>> Not sure this is a good idea because of potential of clashing with
>> extlinux appended command line options.
> 
> globalvar_add_bool("extlinux.fix_root", NULL); ?

global.extlinux.strip_root to remove root=
and bootm.appendroot to add barebox' own root?

@Sascha, any thoughts on this?

> 
> ...
>>> +     if (e->append)
>>> +             globalvar_add_simple("linux.bootargs.dyn.extlinux", e->append);
>>> +
>>> +     pr_info("Booting extlinux label '%s'\n", e->label);
>>> +
>>> +     ret = bootm_boot(&data);
>>
>> Please make use of new bootm_entry instead, so it can be used with devboot:
>>
>> https://github.com/barebox/barebox/blob/next/Documentation/user/devboot.rst
> 
> I haven't figured out how to do this yet...

Just make sure it can be used by calling bootm_entry instead of bootm_boot.


> ...
>>> +static struct extlinux_entry *parse_extlinux_conf(const char *abspath,
>>> +                                               const char *rootpath)
>>> +{
>>> +     char *buf, *bufptr, *line, *p, *default_label = NULL;
>>> +     struct extlinux_entry *entry = NULL;
>>> +
>>> +     bufptr = read_file(abspath, NULL);
>>> +     if (!bufptr)
>>> +             return ERR_PTR(-errno);
>>> +
>>> +     for (p = bufptr; *p; p++)
>>> +             if (*p == '\r')
>>> +                     *p = '\n';
>>
>> I think this can be dropped.
>>
>>> +
>>> +     buf = bufptr;
>>> +     while ((line = strsep(&buf, "\n")) != NULL) {
>>
>> And then you replace the set searched here with "\r\n"
> 
> The end of the line may be \n, \r, \n\r.
> We are simply converting it to a uniform format.

strsep's second argument means split at any of these characters and not
split at them in exactly that order.

> 
>>> +             char *key, *val;
>>> +
>>> +             line = skip_spaces(line);
>>> +
>>> +             if (*line == '#' || *line == '\0')
>>> +                     continue;
>>> +
>>
>> Nitpick: I think below can be simplified to:
>>
>>> +             key = line;
>>> +             val = strchr(line, ' ');
>>> +             if (!val)
>>> +                     val = strchr(line, '\t');
>>> +             if (val) {
>>> +                     *val++ = '\0';
>>> +                     val = skip_spaces(val);
>>> +             } else
>>> +                     continue;
>>
>> key = strsep(&line, ' \t');
>> val = strsep(&line, '\n');
>> val = isempty(val) ? NULL : skip_spaces(val);
> 
> strsep inserts a null byte in place of the separator; this will
> corrupt the buffer,
> and consequently, we will not be able to find the val.

char line[] = "DEFAULT test", *p = line;

char *key = strsep(&p, " \t");
char *val = strsep(&p, "\n");

printf("key<%s> value<%s>\n", key, val);

prints key<DEFAULT> value<test>.
Isn't that exactly what you need?

>> defaut_label might not have been set here?
> 
> No: if (!default_label) ... continue;

Ah ok.

Cheers,
Ahmad

-- 
Pengutronix e.K.                  |                             |
Steuerwalder Str. 21              | http://www.pengutronix.de/  |
31137 Hildesheim, Germany         | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686  | Fax:   +49-5121-206917-5555 |