On 2015-12-16 08:34:20, John Johansen wrote:
> 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.

I considered that but ultimately, I think `aa-status --enabled` should
end up exec'ing aa-enabled. It would be nice if it could simply do that
without worrying about exit status translation.

Tyler

Attachment: signature.asc
Description: Digital signature

-- 
AppArmor mailing list
[email protected]
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/apparmor

Reply via email to