On 12/16/2015 08:13 AM, Tyler Hicks wrote:
> On 2015-12-16 14:07:53, Christian Boltz wrote:
>> Hello,
>>
>> Am Dienstag, 15. Dezember 2015 schrieb Seth Arnold:
>>> On Tue, Dec 15, 2015 at 06:41:48PM -0600, Tyler Hicks wrote:
>>>>> + if (!quiet) {
>>>>> +         switch(err) {
>>>>> +         case ENOSYS:
>>>>> +                 printf(_("No - not available on this system.\n"));
>>>>> +                 break;
>>>>> +         case ECANCELED:
>>>>> +                 printf(_("No - disabled at boot.\n"));
>>>>> +                 break;
>>>>> +         case ENOENT:
>>>>> +                 printf(_("Maybe - policy interface not available.\n"));
>>>>> +                 break;
>>>>> +         case EPERM:
>>>>> +         case EACCES:
>>>>> +                 printf(_("Maybe - insufficient permissions to determine
>>>>> availability.\n")); +                     break;
>>>>> +         default:
>>>>> +           printf(_("Error - '%s'\n"), strerror(err));
>>>>> +         }
>>>>> + }
>>>>> +
>>>>> + return err;
>>>>
>>>> Do we really want to return an errno value here? Why not just
>>>> EXIT_FAILURE?
>>>
>>> Sigh, I looked right at this, made suggestions, and missed the point
>>> entirely -- we have to exit with different exit codes because the exit
>>> code from aa-status(8) is documented with these descriptions. But we
>>> can't just return with EPERM, we actually need to map all these to
>>> 1--4.
>>
>> I mostly agree, however the description of 1..4 in aa-status(8) 
>> describes only "expected" errors. We might want to use a different value 
>> for unexpected errors (that's the "default:" branch in the code quoted 
>> above), and should of course document that additional exit code in the 
>> manpage. (I'd recommend not to use 5 to have some room reserved if we 
>> ever decide to add another "expected" error.)
> 
> Also, the expected error that results in exit status of '2' has nothing
> to do with aa-enabled:
> 
>        2   if apparmor is enabled but no policy is loaded.
> 
> What this translates to is that /sys/kernel/security/apparmor/profiles
> is empty. However, we have no reason to inspect that file in aa-enabled.
> 
> Also, I think it is a bug that `aa-status --enabled` will return 2 if
> the profiles file is empty. Should we change that behavior?
> 
Yes.

Also, I think it is entirely reasonable for aa-enabled to deviate from
the error return scheme we had for aa-status --enabled. EXIT_FAILURE and
EXIT_SUCCESS I think are perfectly acceptable for aa-enabled.



-- 
AppArmor mailing list
AppArmor@lists.ubuntu.com
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/apparmor

Reply via email to