selinux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/1] python/semanage: check rc after getting it
@ 2020-04-19 14:00 Nicolas Iooss
  2020-04-20 14:28 ` Roberts, William C
  0 siblings, 1 reply; 6+ messages in thread
From: Nicolas Iooss @ 2020-04-19 14:00 UTC (permalink / raw)
  To: selinux

This issue has been found using lgtm.com:
https://lgtm.com/projects/g/SELinuxProject/selinux/snapshot/4946f674a6da9cc368cc826f963aedd39b6a94cf/files/python/semanage/seobject.py?sort=name&dir=ASC&mode=heatmap#x5c052fffe98aee02:1

Fixes: 49706ad9f808 ("Revised Patch for local nodecon support in
semanage (was: Adding local nodecon's through semanage)")
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
---
 python/semanage/seobject.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/python/semanage/seobject.py b/python/semanage/seobject.py
index 0e9ce2900892..f2a139c970bd 100644
--- a/python/semanage/seobject.py
+++ b/python/semanage/seobject.py
@@ -1895,10 +1895,10 @@ class nodeRecords(semanageRecords):
         (rc, k) = semanage_node_key_create(self.sh, addr, mask, proto)
         if rc < 0:
             raise ValueError(_("Could not create key for %s") % addr)
-        if rc < 0:
-            raise ValueError(_("Could not check if addr %s is defined") % addr)
 
         (rc, exists) = semanage_node_exists(self.sh, k)
+        if rc < 0:
+            raise ValueError(_("Could not check if addr %s is defined") % addr)
         if exists:
             raise ValueError(_("Addr %s already defined") % addr)
 
-- 
2.26.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* RE: [PATCH 1/1] python/semanage: check rc after getting it
  2020-04-19 14:00 [PATCH 1/1] python/semanage: check rc after getting it Nicolas Iooss
@ 2020-04-20 14:28 ` Roberts, William C
  2020-04-20 18:30   ` Nicolas Iooss
  2020-04-22 21:49   ` Roberts, William C
  0 siblings, 2 replies; 6+ messages in thread
From: Roberts, William C @ 2020-04-20 14:28 UTC (permalink / raw)
  To: Nicolas Iooss, selinux

> -----Original Message-----
> From: selinux-owner@vger.kernel.org [mailto:selinux-owner@vger.kernel.org]
> On Behalf Of Nicolas Iooss
> Sent: Sunday, April 19, 2020 9:01 AM
> To: selinux@vger.kernel.org
> Subject: [PATCH 1/1] python/semanage: check rc after getting it
> 
> This issue has been found using lgtm.com:
> https://lgtm.com/projects/g/SELinuxProject/selinux/snapshot/4946f674a6da9cc3
> 68cc826f963aedd39b6a94cf/files/python/semanage/seobject.py?sort=name&dir
> =ASC&mode=heatmap#x5c052fffe98aee02:1
> 
> Fixes: 49706ad9f808 ("Revised Patch for local nodecon support in semanage (was:
> Adding local nodecon's through semanage)")
> Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
> ---
>  python/semanage/seobject.py | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/python/semanage/seobject.py b/python/semanage/seobject.py
> index 0e9ce2900892..f2a139c970bd 100644
> --- a/python/semanage/seobject.py
> +++ b/python/semanage/seobject.py
> @@ -1895,10 +1895,10 @@ class nodeRecords(semanageRecords):
>          (rc, k) = semanage_node_key_create(self.sh, addr, mask, proto)
>          if rc < 0:
>              raise ValueError(_("Could not create key for %s") % addr)
> -        if rc < 0:
> -            raise ValueError(_("Could not check if addr %s is defined") % addr)
> 
>          (rc, exists) = semanage_node_exists(self.sh, k)
> +        if rc < 0:
> +            raise ValueError(_("Could not check if addr %s is defined")
> + % addr)
>          if exists:
>              raise ValueError(_("Addr %s already defined") % addr)
> 
> --
> 2.26.0

Acked-by: William Roberts <william.c.roberts@intel.com>

We should probably add the checker so it comments on the PRs. It would have caught it
Before it got in tree.

Completely unrelated, do you see these warnings?

swigging selinuxswig_python.i to selinuxswig_python_wrap.c
swig -python -o selinuxswig_python_wrap.c selinuxswig_python.i
../include/selinux/avc.h:414: Warning 302: Identifier 'avc_cache_stats' redefined (ignored),
../include/selinux/avc.h:394: Warning 302: previous definition of 'avc_cache_stats'.
../include/selinux/selinux.h:143: Warning 451: Setting a const char * variable may leak memory.
../include/selinux/selinux.h:378: Warning 451: Setting a const char * variable may leak memory.

I created a ticket, so I don't lose them:
https://github.com/SELinuxProject/selinux/issues/222



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/1] python/semanage: check rc after getting it
  2020-04-20 14:28 ` Roberts, William C
@ 2020-04-20 18:30   ` Nicolas Iooss
  2020-04-20 18:37     ` Christian Göttsche
  2020-04-22 21:49   ` Roberts, William C
  1 sibling, 1 reply; 6+ messages in thread
From: Nicolas Iooss @ 2020-04-20 18:30 UTC (permalink / raw)
  To: Roberts, William C; +Cc: selinux

On Mon, Apr 20, 2020 at 4:29 PM Roberts, William C
<william.c.roberts@intel.com> wrote:
>
> > -----Original Message-----
> > From: selinux-owner@vger.kernel.org [mailto:selinux-owner@vger.kernel.org]
> > On Behalf Of Nicolas Iooss
> > Sent: Sunday, April 19, 2020 9:01 AM
> > To: selinux@vger.kernel.org
> > Subject: [PATCH 1/1] python/semanage: check rc after getting it
> >
> > This issue has been found using lgtm.com:
> > https://lgtm.com/projects/g/SELinuxProject/selinux/snapshot/4946f674a6da9cc3
> > 68cc826f963aedd39b6a94cf/files/python/semanage/seobject.py?sort=name&dir
> > =ASC&mode=heatmap#x5c052fffe98aee02:1
> >
> > Fixes: 49706ad9f808 ("Revised Patch for local nodecon support in semanage (was:
> > Adding local nodecon's through semanage)")
> > Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
> > ---
> >  python/semanage/seobject.py | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/python/semanage/seobject.py b/python/semanage/seobject.py
> > index 0e9ce2900892..f2a139c970bd 100644
> > --- a/python/semanage/seobject.py
> > +++ b/python/semanage/seobject.py
> > @@ -1895,10 +1895,10 @@ class nodeRecords(semanageRecords):
> >          (rc, k) = semanage_node_key_create(self.sh, addr, mask, proto)
> >          if rc < 0:
> >              raise ValueError(_("Could not create key for %s") % addr)
> > -        if rc < 0:
> > -            raise ValueError(_("Could not check if addr %s is defined") % addr)
> >
> >          (rc, exists) = semanage_node_exists(self.sh, k)
> > +        if rc < 0:
> > +            raise ValueError(_("Could not check if addr %s is defined")
> > + % addr)
> >          if exists:
> >              raise ValueError(_("Addr %s already defined") % addr)
> >
> > --
> > 2.26.0
>
> Acked-by: William Roberts <william.c.roberts@intel.com>
>
> We should probably add the checker so it comments on the PRs. It would have caught it
> Before it got in tree.

In this case, the bug has been introduced in 2008 ;) Anyway, I have
never configured lgtm.com to look on PRs, or contributed to a project
that did this. For SELinux, I am wondering whether the hundreds of
alerts currently reported in
https://lgtm.com/projects/g/SELinuxProject/selinux/?mode=list could
affect the usefulness of having it integrated to PRs.

> Completely unrelated, do you see these warnings?
>
> swigging selinuxswig_python.i to selinuxswig_python_wrap.c
> swig -python -o selinuxswig_python_wrap.c selinuxswig_python.i
> ../include/selinux/avc.h:414: Warning 302: Identifier 'avc_cache_stats' redefined (ignored),
> ../include/selinux/avc.h:394: Warning 302: previous definition of 'avc_cache_stats'.
> ../include/selinux/selinux.h:143: Warning 451: Setting a const char * variable may leak memory.
> ../include/selinux/selinux.h:378: Warning 451: Setting a const char * variable may leak memory.
>
> I created a ticket, so I don't lose them:
> https://github.com/SELinuxProject/selinux/issues/222

Yes, and I see that an explanation has already been given in the GitHub issue.

Cheers,
Nicolas


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/1] python/semanage: check rc after getting it
  2020-04-20 18:30   ` Nicolas Iooss
@ 2020-04-20 18:37     ` Christian Göttsche
  2020-04-20 22:24       ` William Roberts
  0 siblings, 1 reply; 6+ messages in thread
From: Christian Göttsche @ 2020-04-20 18:37 UTC (permalink / raw)
  To: Nicolas Iooss; +Cc: Roberts, William C, selinux

Am Mo., 20. Apr. 2020 um 20:30 Uhr schrieb Nicolas Iooss
<nicolas.iooss@m4x.org>:
>
> On Mon, Apr 20, 2020 at 4:29 PM Roberts, William C
> <william.c.roberts@intel.com> wrote:
> >
> > We should probably add the checker so it comments on the PRs. It would have caught it
> > Before it got in tree.
>
> In this case, the bug has been introduced in 2008 ;) Anyway, I have
> never configured lgtm.com to look on PRs, or contributed to a project
> that did this. For SELinux, I am wondering whether the hundreds of
> alerts currently reported in
> https://lgtm.com/projects/g/SELinuxProject/selinux/?mode=list could
> affect the usefulness of having it integrated to PRs.

I think lgtm.com only shows new issues on pull requests.
For example systemd uses it and I never got an unrelated alert.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/1] python/semanage: check rc after getting it
  2020-04-20 18:37     ` Christian Göttsche
@ 2020-04-20 22:24       ` William Roberts
  0 siblings, 0 replies; 6+ messages in thread
From: William Roberts @ 2020-04-20 22:24 UTC (permalink / raw)
  To: Christian Göttsche; +Cc: Nicolas Iooss, Roberts, William C, selinux

On Mon, Apr 20, 2020 at 1:38 PM Christian Göttsche
<cgzones@googlemail.com> wrote:
>
> Am Mo., 20. Apr. 2020 um 20:30 Uhr schrieb Nicolas Iooss
> <nicolas.iooss@m4x.org>:
> >
> > On Mon, Apr 20, 2020 at 4:29 PM Roberts, William C
> > <william.c.roberts@intel.com> wrote:
> > >
> > > We should probably add the checker so it comments on the PRs. It would have caught it
> > > Before it got in tree.
> >
> > In this case, the bug has been introduced in 2008 ;) Anyway, I have

For some reason, given the timing I thought I missed something in the review of
the other patches. I guess I should have used git blame vs assuming!

> > never configured lgtm.com to look on PRs, or contributed to a project
> > that did this. For SELinux, I am wondering whether the hundreds of
> > alerts currently reported in
> > https://lgtm.com/projects/g/SELinuxProject/selinux/?mode=list could
> > affect the usefulness of having it integrated to PRs.
>
> I think lgtm.com only shows new issues on pull requests.
> For example systemd uses it and I never got an unrelated alert.

Exactly, it only gripes if you add. So they will help keep the list
from growing at least.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH 1/1] python/semanage: check rc after getting it
  2020-04-20 14:28 ` Roberts, William C
  2020-04-20 18:30   ` Nicolas Iooss
@ 2020-04-22 21:49   ` Roberts, William C
  1 sibling, 0 replies; 6+ messages in thread
From: Roberts, William C @ 2020-04-22 21:49 UTC (permalink / raw)
  To: 'Nicolas Iooss', 'selinux@vger.kernel.org'

> -----Original Message-----
> From: Roberts, William C
> Sent: Monday, April 20, 2020 9:29 AM
> To: Nicolas Iooss <nicolas.iooss@m4x.org>; selinux@vger.kernel.org
> Subject: RE: [PATCH 1/1] python/semanage: check rc after getting it
> 
> > -----Original Message-----
> > From: selinux-owner@vger.kernel.org
> > [mailto:selinux-owner@vger.kernel.org]
> > On Behalf Of Nicolas Iooss
> > Sent: Sunday, April 19, 2020 9:01 AM
> > To: selinux@vger.kernel.org
> > Subject: [PATCH 1/1] python/semanage: check rc after getting it
> >
> > This issue has been found using lgtm.com:
> > https://lgtm.com/projects/g/SELinuxProject/selinux/snapshot/4946f674a6
> > da9cc3
> > 68cc826f963aedd39b6a94cf/files/python/semanage/seobject.py?sort=name&d
> > ir
> > =ASC&mode=heatmap#x5c052fffe98aee02:1
> >
> > Fixes: 49706ad9f808 ("Revised Patch for local nodecon support in semanage
> (was:
> > Adding local nodecon's through semanage)")
> > Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
> > ---
> >  python/semanage/seobject.py | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/python/semanage/seobject.py b/python/semanage/seobject.py
> > index 0e9ce2900892..f2a139c970bd 100644
> > --- a/python/semanage/seobject.py
> > +++ b/python/semanage/seobject.py
> > @@ -1895,10 +1895,10 @@ class nodeRecords(semanageRecords):
> >          (rc, k) = semanage_node_key_create(self.sh, addr, mask, proto)
> >          if rc < 0:
> >              raise ValueError(_("Could not create key for %s") % addr)
> > -        if rc < 0:
> > -            raise ValueError(_("Could not check if addr %s is defined") % addr)
> >
> >          (rc, exists) = semanage_node_exists(self.sh, k)
> > +        if rc < 0:
> > +            raise ValueError(_("Could not check if addr %s is
> > + defined") % addr)
> >          if exists:
> >              raise ValueError(_("Addr %s already defined") % addr)
> >
> > --
> > 2.26.0
> 
> Acked-by: William Roberts <william.c.roberts@intel.com>
> 
> We should probably add the checker so it comments on the PRs. It would have
> caught it Before it got in tree.
> 
> Completely unrelated, do you see these warnings?
> 
> swigging selinuxswig_python.i to selinuxswig_python_wrap.c swig -python -o
> selinuxswig_python_wrap.c selinuxswig_python.i
> ../include/selinux/avc.h:414: Warning 302: Identifier 'avc_cache_stats' redefined
> (ignored),
> ../include/selinux/avc.h:394: Warning 302: previous definition of
> 'avc_cache_stats'.
> ../include/selinux/selinux.h:143: Warning 451: Setting a const char * variable may
> leak memory.
> ../include/selinux/selinux.h:378: Warning 451: Setting a const char * variable may
> leak memory.
> 
> I created a ticket, so I don't lose them:
> https://github.com/SELinuxProject/selinux/issues/222
> 

Merged:
https://github.com/SELinuxProject/selinux/pull/223

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-04-22 21:49 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-19 14:00 [PATCH 1/1] python/semanage: check rc after getting it Nicolas Iooss
2020-04-20 14:28 ` Roberts, William C
2020-04-20 18:30   ` Nicolas Iooss
2020-04-20 18:37     ` Christian Göttsche
2020-04-20 22:24       ` William Roberts
2020-04-22 21:49   ` Roberts, William C

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).