Lennart Poettering <lenn...@poettering.net> writes:
> On Fri, 07.11.14 15:18, Jan Synacek (jsyna...@redhat.com) 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 systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
