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 <peben...@ieee.org> 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.khodorkovs...@crunchydata.com> 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 <y...@crunchydata.com> > >>> --- > >>> 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@tycho.nsa.gov To unsubscribe, send email to selinux-le...@tycho.nsa.gov. To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.