Re: changelog files, %( %) idioms

[Ananth N Mavinakayanahalli <ananth at in dot ibm dot com>]

On Mon, Feb 25, 2008 at 09:59:15AM -0500, Frank Ch. Eigler wrote:
> 
> Thanks for the bug #5772 patch.  It is possible to make the %( kernel
> %) conditionals look more compact by realizing that they don't operate
> at the statement but at the token level.  That means one can use them
> around just the smallest bit of code that needs to be changed for
> different versions/architectures, so that instead of:
> 
>    %( kernel_vr > "2.6.24" %?
>    argstr = sprintf("%d, %p, %s, %p", $upid, $stat_addr, _wait4_opt_str($options), $ru)
>    %:
>    argstr = sprintf("%d, %p, %s, %p", $pid, $stat_addr, _wait4_opt_str($options), $ru)
>    %)
> 
> and
> 
>    %( kernel_vr > "2.6.24" %?
>       pid = $upid
>    %:
>       pid = $pid
>    %)
> 
> one could write ...
> 
>    argstr = sprintf("%d, %p, %s, %p",
>      %( kernel_vr > "2.6.24" %? $upid %: $pid %),
>      $stat_addr, _wait4_opt_str($options), $ru)
> 
> and
> 
>    pid = %( kernel_vr > "2.6.24" %? $upid %: $pid %)

We have a similar issue with the x86_32 syscalls tapset. sys_signalstack
has a reference to ebx, while with register unification, its now bx. The
tapset is thus broken for kernel_vr > 2.6.24.

Following the compact method, the patch looks thus:

---
 tapset/i686/syscalls.stp |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

Index: systemtap-3mar/tapset/i686/syscalls.stp
===================================================================
--- systemtap-3mar.orig/tapset/i686/syscalls.stp
+++ systemtap-3mar/tapset/i686/syscalls.stp
@@ -119,8 +119,8 @@ probe syscall.set_zone_reclaim.return =
 #
 probe syscall.sigaltstack = kernel.function("sys_sigaltstack") {
 	name = "sigaltstack"
-	ebx = $ebx
-	argstr = sprintf("%p", $ebx)
+	bx = %( kernel_vr > "2.6.24" %? $bx %: $ebx %)
+	argstr = sprintf("%p", %( kernel_vr > "2.6.24" %? $bx %: $ebx %) )
 }
 probe syscall.sigaltstack.return = kernel.function("sys_sigaltstack").return {
 	name = "sigaltstack"

Notice that the LHS has changed from ebx to bx.

However, if we need to have a 100% backward compatibility, I'd prefer
the older approach thus:

---
 tapset/i686/syscalls.stp |    5 +++++
 1 files changed, 5 insertions(+)

Index: systemtap-3mar/tapset/i686/syscalls.stp
===================================================================
--- systemtap-3mar.orig/tapset/i686/syscalls.stp
+++ systemtap-3mar/tapset/i686/syscalls.stp
@@ -119,8 +119,13 @@ probe syscall.set_zone_reclaim.return =
 #
 probe syscall.sigaltstack = kernel.function("sys_sigaltstack") {
 	name = "sigaltstack"
+%( kernel_vr > "2.6.24" %?
+	bx = $bx
+	argstr = sprintf("%p", $bx)
+%:
 	ebx = $ebx
 	argstr = sprintf("%p", $ebx)
+%)
 }
 probe syscall.sigaltstack.return = kernel.function("sys_sigaltstack").return {
 	name = "sigaltstack"

Comments?

Ananth