Re: [PATCH] libsemanage: improve semanage_migrate_store import failure

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Weird Gmail removed my text box for plain text mode in Gmail,
re-sending since it got
filtered out of the mailing list.

On Mon, Oct 8, 2018 at 9:09 AM William Roberts <bill.c.roberts@xxxxxxxxx> wrote:
>
> Yuli,
> If you respin this with just import error looks like its a go.
> Bill
>
> On Fri, Oct 5, 2018 at 12:53 PM Chris PeBenito <pebenito@xxxxxxxx> wrote:
>>
>> On 10/05/2018 10:32 AM, Jason Zaman wrote:
>> > On Fri, Oct 05, 2018 at 07:13:23AM -0700, William Roberts wrote:
>> >> On Thu, Oct 4, 2018 at 12:46 PM Yuli Khodorkovskiy <
>> >> yuli.khodorkovskiy@xxxxxxxxxxxxxxx> wrote:
>> >>
>> >>> The python module import error in semanage_migrate_store was misleading.
>> >>> Before, it would print that the module is not installed, even though
>> >>> it is in fact on the system.
>> >>>
>> >>> Now the python module import failure is correctly reported if the module
>> >>> is not installed or the exact reason for failure is reported to the user.
>> >>>
>> >>> Signed-off-by: Yuli Khodorkovskiy <yuli@xxxxxxxxxxxxxxx>
>> >>> ---
>> >>>   libsemanage/utils/semanage_migrate_store | 6 ++++--
>> >>>   1 file changed, 4 insertions(+), 2 deletions(-)
>> >>>
>> >>> diff --git a/libsemanage/utils/semanage_migrate_store
>> >>> b/libsemanage/utils/semanage_migrate_store
>> >>> index 2e6cb278..50eb59ef 100755
>> >>> --- a/libsemanage/utils/semanage_migrate_store
>> >>> +++ b/libsemanage/utils/semanage_migrate_store
>> >>> @@ -15,10 +15,12 @@ sepol = ctypes.cdll.LoadLibrary('libsepol.so.1')
>> >>>   try:
>> >>>          import selinux
>> >>>          import semanage
>> >>> -except:
>> >>> +except ImportError:
>> >>>          print("You must install libselinux-python and libsemanage-python
>> >>> before running this tool", file=sys.stderr)
>> >>>          exit(1)
>> >>> -
>> >>> +except Exception as e:
>> >>> +       print("Failed to import libselinux-python/libsemanage-python: %s"
>> >>> % str(e))
>> >>> +       exit(1)
>> >>>
>> >>
>> >> We should really only be handling exceptions we reasonably expect and
>> >> discourage
>> >> the usage of catching raw Exception, especially considering not-catching
>> >> this will
>> >> cause the runtime to print a stack trace, the error and exit non-zero.
>> >>
>> >> We probably only need the except ImportError change and can drop the second
>> >> hunk.
>> >>
>> >> Does anyone disagree with this?
>>
>> For this case, I agree that ImportError is the only thing that should be
>> caught.
>>
>> > Agreed. catching Exception is bad cuz it also catches KeyboardInterrupt
>> > and stuff like that.
>>
>> That's not correct.  Catching BaseException would catch
>> KeyboardInterrupt.  Catching Exception would not.  See the Python
>> builtin exception hierarchy:
>>
>> https://docs.python.org/3/library/exceptions.html#exception-hierarchy
>>
>> IMO catching Exception has valid uses.
>>
>> --
>> Chris PeBenito
_______________________________________________
Selinux mailing list
Selinux@xxxxxxxxxxxxx
To unsubscribe, send email to Selinux-leave@xxxxxxxxxxxxx.
To get help, send an email containing "help" to Selinux-request@xxxxxxxxxxxxx.



[Index of Archives]     [Selinux Refpolicy]     [Linux SGX]     [Fedora Users]     [Fedora Desktop]     [Yosemite Photos]     [Yosemite Camping]     [Yosemite Campsites]     [KDE Users]     [Gnome Users]

  Powered by Linux