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.
