[libvirt] [PATCH] Add a rule to check for uses of readlink.
Signed-off-by: Chris Lalancette <clalance@redhat.com> --- .x-sc_prohibit_readlink | 2 ++ cfg.mk | 5 +++++ 2 files changed, 7 insertions(+), 0 deletions(-) create mode 100644 .x-sc_prohibit_readlink diff --git a/.x-sc_prohibit_readlink b/.x-sc_prohibit_readlink new file mode 100644 index 0000000..e7acb03 --- /dev/null +++ b/.x-sc_prohibit_readlink @@ -0,0 +1,2 @@ +^src/util/util\.c$ +^ChangeLog-old$ diff --git a/cfg.mk b/cfg.mk index 0f2d2a6..5013e4b 100644 --- a/cfg.mk +++ b/cfg.mk @@ -94,6 +94,11 @@ sc_prohibit_strncpy: msg='use virStrncpy, not strncpy' \ $(_prohibit_regexp) +sc_prohibit_readlink: + @re='readlink *\(' \ + msg='use virFileResolveLink, not readlink' \ + $(_prohibit_regexp) + sc_prohibit_gethostname: @re='gethostname *\(' \ msg='use virGetHostname, not gethostname' \ -- 1.6.6
On Thu, 2010-01-21 at 11:33 -0500, Chris Lalancette wrote:
Signed-off-by: Chris Lalancette <clalance@redhat.com> --- .x-sc_prohibit_readlink | 2 ++ cfg.mk | 5 +++++ 2 files changed, 7 insertions(+), 0 deletions(-) create mode 100644 .x-sc_prohibit_readlink
This breaks AppArmor (see why in my response to the AppArmor change). Readlink() can be used safely, so perhaps the check can be done such that if using readlink, you must check the return code. Or simply warn if using readlink. virFileResolveLink() behaves substantially differently than readlink() and deprecating readlink() without adjusting virFileResolveLink() is IMHO unwise (while AppArmor is the only thing affected atm, it seems at least possible that new future code may need/want to readlink() things in /proc (eg /proc/self/exe)). Jamie -- Jamie Strandboge | http://www.canonical.com
On Thu, Jan 21, 2010 at 11:00:58AM -0600, Jamie Strandboge wrote:
On Thu, 2010-01-21 at 11:33 -0500, Chris Lalancette wrote:
Signed-off-by: Chris Lalancette <clalance@redhat.com> --- .x-sc_prohibit_readlink | 2 ++ cfg.mk | 5 +++++ 2 files changed, 7 insertions(+), 0 deletions(-) create mode 100644 .x-sc_prohibit_readlink
This breaks AppArmor (see why in my response to the AppArmor change). Readlink() can be used safely, so perhaps the check can be done such that if using readlink, you must check the return code. Or simply warn if using readlink.
virFileResolveLink() behaves substantially differently than readlink() and deprecating readlink() without adjusting virFileResolveLink() is IMHO unwise (while AppArmor is the only thing affected atm, it seems at least possible that new future code may need/want to readlink() things in /proc (eg /proc/self/exe)).
Jamie
I understand this as being resolved by gnulib implementation, in which case ACK to the make check addition, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
On 01/22/2010 09:44 AM, Daniel Veillard wrote:
On Thu, Jan 21, 2010 at 11:00:58AM -0600, Jamie Strandboge wrote:
On Thu, 2010-01-21 at 11:33 -0500, Chris Lalancette wrote:
Signed-off-by: Chris Lalancette <clalance@redhat.com> --- .x-sc_prohibit_readlink | 2 ++ cfg.mk | 5 +++++ 2 files changed, 7 insertions(+), 0 deletions(-) create mode 100644 .x-sc_prohibit_readlink
This breaks AppArmor (see why in my response to the AppArmor change). Readlink() can be used safely, so perhaps the check can be done such that if using readlink, you must check the return code. Or simply warn if using readlink.
virFileResolveLink() behaves substantially differently than readlink() and deprecating readlink() without adjusting virFileResolveLink() is IMHO unwise (while AppArmor is the only thing affected atm, it seems at least possible that new future code may need/want to readlink() things in /proc (eg /proc/self/exe)).
Jamie
I understand this as being resolved by gnulib implementation, in which case ACK to the make check addition,
Right, exactly. Thanks, pushed now. -- Chris Lalancette
participants (3)
-
Chris Lalancette -
Daniel Veillard -
Jamie Strandboge