Lennart Poettering <[email protected]> writes: > On Fri, 07.11.14 15:18, Jan Synacek ([email protected]) wrote: > >> } >> if (!isempty(state)) >> log_syntax(unit, LOG_ERR, filename, line, EINVAL, >> @@ -1043,7 +1049,8 @@ static int unit_file_load( >> const char *path, >> const char *root_dir, >> bool allow_symlink, >> - bool load) { >> + bool load, >> + char ***also) { > > Hmm, do we really want to return the full list here? I don't think any > caller really is interested in that, or am I wrong? Wouldn't a bool* > suffice here that tells us if also is empty, that we fill in?
No, it's not necessary. I'm not sure why I wrote it that way. Let's
blame it on friday...
> I mean, I think the inner calls should parse the whole strv, i see no
> problem with that, I just don't think we really need to pass the whole
> thing all the way up...
>
>> const ConfigTableItem items[] = {
>> { "Install", "Alias", config_parse_strv,
>> 0, &info->aliases },
>> @@ -1087,6 +1094,9 @@ static int unit_file_load(
>> if (r < 0)
>> return r;
>>
>> + if (also && !strv_isempty(info->also))
>> + *also = strv_copy(info->also);
>> +
>
> If the argument would just be a bool* we wouldn't have to do the
> expensive strv_copy() here... (which is missing an OOM check btw...)
>
> Otherwise looks great!
>
> Lennart
Next version incoming!
Cheers,
--
Jan Synacek
Software Engineer, Red Hat
signature.asc
Description: PGP signature
_______________________________________________ systemd-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/systemd-devel
