| Message ID | 20200419140055.86159-1-nicolas.iooss@m4x.org (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [1/1] python/semanage: check rc after getting it | expand |
> -----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
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
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.
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.
> -----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
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)
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(-)