[systemd-devel] [PATCH] shared/install: when unit contains only Also=, report 'indirect'

[Jan Synacek]

Lennart Poettering <lennart at poettering.net> writes:
> On Fri, 07.11.14 15:18, Jan Synacek (jsynacek at 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 818 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/systemd-devel/attachments/20141107/c41fc223/attachment.sig>