Bug 9871 - use @cast() instead of embedded-c whereever possible
Summary: use @cast() instead of embedded-c whereever possible
Status: RESOLVED FIXED
Alias: None
Product: systemtap
Classification: Unclassified
Component: tapsets (show other bugs)
Version: unspecified
: P2 normal
Target Milestone: ---
Assignee: Unassigned
URL:
Keywords:
Depends on: 9932
Blocks:
  Show dependency treegraph
 
Reported: 2009-02-19 18:22 UTC by Frank Ch. Eigler
Modified: 2010-09-27 14:45 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:


Attachments
Revamp some functions of nfs.stp using @cast (513 bytes, text/plain)
2009-03-05 08:05 UTC, Wenji Huang
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Frank Ch. Eigler 2009-02-19 18:22:52 UTC
Now that we have it, let's use it.
Comment 1 Mark Wielaard 2009-02-27 14:46:30 UTC
I did one small rewrite of tapset/inet_sock.stp, which is nicely smaller now. It
does still need a small tidbit of embedded C because we cannot easily cast
between basic types/unions, see the new daddr_to_string.

function inet_get_local_port:long(sock:long)
{
%(kernel_v < "2.6.21" %?
  port = @cast(sock, "inet_sock", "kernel")->inet->num;
%:
  port = @cast(sock, "inet_sock", "kernel")->num;
%)
  return port;
}

// Get IP source address string given a pointer to a kernel socket.
function inet_get_ip_source:string(sock:long)
{
%(kernel_v < "2.6.21" %?
  daddr = @cast(sock, "inet_sock", "kernel")->inet->daddr;
%:
  daddr = @cast(sock, "inet_sock", "kernel")->daddr;
%)
  return daddr_to_string(daddr);
}

// Turns a daddr as found in an inet_sock into a dotted ip string.
function daddr_to_string:string(daddr:long)
%{ /* pure */
	union { __u32 d; unsigned char addr[4]; } u;
	u.d = THIS->daddr;
	sprintf(THIS->__retvalue, "%d.%d.%d.%d",
			u.addr[0], u.addr[1], u.addr[2], u.addr[3]);
%}
Comment 2 Wenji Huang 2009-03-05 08:05:45 UTC
Created attachment 3794 [details]
Revamp some functions of nfs.stp using @cast 

The new functions look more compact and many duplicated codes have been
eliminated.
Comment 3 Mark Wielaard 2009-03-05 12:12:42 UTC
Yes much nicer and much more readable compared to the previous versions. Thanks.

One minor stylistic request. Could you add spaces around the ? and : so that
something like:

dentry = file? @cast(file, "file", "kernel")->f_dentry: 0

will look like:

dentry = file ? @cast(file, "file", "kernel")->f_dentry : 0

that makes them a bit more readable and makes the sources a bit more consistent
(currently the sources sometimes do and sometimes don't add spaces).
Comment 4 Wenji Huang 2009-03-06 09:48:48 UTC
thanks for your review. More update see commit
fb3b52a7346202fea1905ed680a3256d372a7b03 PR9871: use @cast in tapset 
Comment 5 Jim Keniston 2009-03-09 20:09:23 UTC
In taspset/*/registers.stp, _stp_get_register_by_offset() could be replaced
using @cast.
Comment 6 Josh Stone 2009-03-09 20:38:30 UTC
(In reply to comment #5)
> In taspset/*/registers.stp, _stp_get_register_by_offset() could be replaced
> using @cast.

Just remember that @cast requires debuginfo -- aren't the registers tapsets
designed to be helpful for running with no debuginfo?
Comment 7 Jim Keniston 2009-03-09 21:06:11 UTC
Correct.  (In reply to comment #6)
> (In reply to comment #5)
> > In taspset/*/registers.stp, _stp_get_register_by_offset() could be replaced
> > using @cast.
> 
> Just remember that @cast requires debuginfo -- aren't the registers tapsets
> designed to be helpful for running with no debuginfo?

Yes, indeed.  Does it help that we'd be casting only to predefined types --
e.g., pointer-to-long?
Comment 8 Josh Stone 2009-03-09 22:27:22 UTC
(In reply to comment #7)
> Yes, indeed.  Does it help that we'd be casting only to predefined types --
> e.g., pointer-to-long?

For POD types, you can just use kernel_long() and family.
Comment 9 Malte Nuhn 2009-04-29 16:37:31 UTC
I have some problems with the 0.9.7 Version of Systemtap.
Scripts than run with 0.9 don't run anymore on 0.9.7.

I think it affects the "@cast" stuff, in my example this affects "inet_sock.stp"

Changing:
daddr = @cast(sock, "inet_sock", "kernel")->inet->daddr;

To:
daddr = @cast(sock, "inet_sock", "kernel<linux/ip.h>")->inet->daddr;

Fixes the problem.

I have a 2.6.9 Kernel.
Comment 10 David Smith 2010-03-26 18:26:47 UTC
Commit 961479c removes some embedded-C from tcpmib.stp
Comment 11 David Smith 2010-03-30 20:57:08 UTC
Commit 1713a05 removed some embedded-C in ioblock.stp/vfs.stp
Comment 12 David Smith 2010-04-05 17:30:31 UTC
Commit fd81a55 removed some embedded-C from nfs_proc.stp.
Comment 13 David Smith 2010-04-05 19:56:00 UTC
Commit abf0244 removed more embedded-C from nfs_proc.stp.
Comment 14 David Smith 2010-04-06 15:05:42 UTC
Commit 84b869b removed more embedded-C in nfs_proc.stp and nfs.stp.
Comment 15 David Smith 2010-04-06 16:37:43 UTC
Commit dd3d6ed removed some embedded-C from scsi.stp.
Comment 16 David Smith 2010-04-07 14:44:11 UTC
Commit 7dfee5e removes all embedded-C in rpc.stp.
Comment 17 David Smith 2010-04-07 21:14:22 UTC
Commit 354c797 removed some embedded-C in ioblock.stp and ipmib.stp.
Comment 18 David Smith 2010-05-11 18:00:23 UTC
Commit 5ccccbb removed all remaining embedded-C from vfs.stp.
Comment 19 David Smith 2010-05-12 17:51:08 UTC
Commit 00bd540 removed some embedded-C from task.stp and context.stp.
Comment 20 David Smith 2010-09-27 14:45:59 UTC
I've made a pass through the tapsets and removed embedded-C when feasible.