> - use err/errx/warn, instead of PERROR, perror and fprintf
Good stuff! I like this.
> - Match () place ment consistent in this files
Thanks for doing this. While you're at it, would you like to fix the bracing?
Some of it is in K&R style:
if ( aoeuaoeu ) {
Whereas other Xen code uses:
if ( aoeuaoeu )
{
Braces always starting on a new line, for all types of block.
> - Use consistent tabs and whitespacing.
Thanks for doing this.
Cheers,
Mark
>
> Signed-off-by: Tony Breeds <tony@xxxxxxxxxxxxxxxxxx>
> ---
>
> tools/xentrace/xentrace.c | 137
> ++++++++++++++--------------------------- tools/xentrace/xentrace_format |
> 51 +++++++--------
> 2 files changed, 73 insertions(+), 115 deletions(-)
>
> Index: xen-unstable.hg-mainline.xentrace/tools/xentrace/xentrace.c
> ===================================================================
> --- xen-unstable.hg-mainline.xentrace.orig/tools/xentrace/xentrace.c
> +++ xen-unstable.hg-mainline.xentrace/tools/xentrace/xentrace.c
> @@ -22,6 +22,7 @@
> #include <signal.h>
> #include <inttypes.h>
> #include <string.h>
> +#include <err.h>
>
> #include <xen/xen.h>
> #include <xen/trace.h>
> @@ -42,16 +43,6 @@
>
> #define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
>
> -#define PERROR(_m, _a...) \
> -do { \
> - int __saved_errno = errno; \
> - fprintf(stderr, "ERROR: " _m " (%d = %s)\n" , ## _a , \
> - __saved_errno, strerror(__saved_errno)); \
> - errno = __saved_errno; \
> -} while (0)
> -
> -extern FILE *stderr;
> -
> /***** Compile time configuration of defaults
> ********************************/
>
> /* when we've got more records than this waiting, we log it to the output
> */ @@ -88,7 +79,7 @@ void close_handler(int signal)
> struct timespec millis_to_timespec(unsigned long millis)
> {
> struct timespec spec;
> -
> +
> spec.tv_sec = millis / 1000;
> spec.tv_nsec = (millis % 1000) * 1000;
>
> @@ -128,10 +119,7 @@ void write_rec(uint32_t cpu, struct t_re
> }
>
> if ( written != 8 )
> - {
> - PERROR("Failed to write trace record");
> - exit(EXIT_FAILURE);
> - }
> + err(EXIT_FAILURE, "Failed to write trace record");
> }
>
> static void get_tbufs(unsigned long *mfn, unsigned long *size)
> @@ -139,21 +127,16 @@ static void get_tbufs(unsigned long *mfn
> int xc_handle = xc_interface_open();
> int ret;
>
> - if ( xc_handle < 0 )
> - {
> - exit(EXIT_FAILURE);
> - }
> + if ( xc_handle < 0 )
> + errx(EXIT_FAILURE, "Unable to open the xc interface");
>
> - if(!opts.tbuf_size)
> - opts.tbuf_size = DEFAULT_TBUF_SIZE;
> + if ( !opts.tbuf_size )
> + opts.tbuf_size = DEFAULT_TBUF_SIZE;
>
> ret = xc_tbuf_enable(xc_handle, opts.tbuf_size, mfn, size);
>
> if ( ret != 0 )
> - {
> - perror("Couldn't enable trace buffers");
> - exit(1);
> - }
> + err(EXIT_FAILURE, "Couldn't enable trace buffers");
>
> xc_interface_close(xc_handle);
> }
> @@ -174,10 +157,8 @@ struct t_buf *map_tbufs(unsigned long tb
>
> xc_handle = xc_interface_open();
>
> - if ( xc_handle < 0 )
> - {
> - exit(EXIT_FAILURE);
> - }
> + if ( xc_handle < 0 )
> + errx(EXIT_FAILURE, "Unable to open the xc interface");
>
> /* On PPC (At least) the DOMID arg is ignored in dom0 */
> tbufs_mapped = xc_map_foreign_range(xc_handle, DOMID_XEN,
> @@ -186,18 +167,15 @@ struct t_buf *map_tbufs(unsigned long tb
>
> xc_interface_close(xc_handle);
>
> - if ( tbufs_mapped == 0 )
> - {
> - PERROR("Failed to mmap trace buffers");
> - exit(EXIT_FAILURE);
> - }
> + if ( tbufs_mapped == 0 )
> + err(EXIT_FAILURE, "Failed to mmap trace buffers");
>
> return tbufs_mapped;
> }
>
> /**
> * set_mask - set the cpu/event mask in HV
> - * @mask: the new mask
> + * @mask: the new mask
> * @type: the new mask type,0-event mask, 1-cpu mask
> *
> */
> @@ -206,21 +184,19 @@ void set_mask(uint32_t mask, int type)
> int ret = 0;
> int xc_handle = xc_interface_open(); /* for accessing control
> interface */
>
> - if (type == 1) {
> + if ( type == 1 ) {
> ret = xc_tbuf_set_cpu_mask(xc_handle, mask);
> - fprintf(stderr, "change cpumask to 0x%x\n", mask);
> - } else if (type == 0) {
> + warnx("change cpumask to 0x%x\n", mask);
> + } else if ( type == 0 ) {
> ret = xc_tbuf_set_evt_mask(xc_handle, mask);
> - fprintf(stderr, "change evtmask to 0x%x\n", mask);
> + warnx("change evtmask to 0x%x\n", mask);
> }
>
> xc_interface_close(xc_handle);
>
> if ( ret != 0 )
> - {
> - PERROR("Failure to get trace buffer pointer from Xen and set the
> new mask"); - exit(EXIT_FAILURE);
> - }
> + err(EXIT_FAILURE, "Failure to get trace buffer pointer from "
> + "Xen and set the new mask");
> }
>
> /**
> @@ -240,11 +216,8 @@ struct t_buf **init_bufs_ptrs(void *bufs
>
> user_ptrs = (struct t_buf **)calloc(num, sizeof(struct t_buf *));
> if ( user_ptrs == NULL )
> - {
> - PERROR( "Failed to allocate memory for buffer pointers\n");
> - exit(EXIT_FAILURE);
> - }
> -
> + err(EXIT_FAILURE, "Failed to allocate memory for buffer
> pointers"); +
> /* initialise pointers to the trace buffers - given the size of a
> trace * buffer and the value of bufs_maped, we can easily calculate these
> */ for ( i = 0; i<num; i++ )
> @@ -269,13 +242,10 @@ struct t_rec **init_rec_ptrs(struct t_bu
> {
> int i;
> struct t_rec **data;
> -
> +
> data = calloc(num, sizeof(struct t_rec *));
> if ( data == NULL )
> - {
> - PERROR("Failed to allocate memory for data pointers\n");
> - exit(EXIT_FAILURE);
> - }
> + err(EXIT_FAILURE, "Failed to allocate memory for data pointers");
>
> for ( i = 0; i < num; i++ )
> data[i] = (struct t_rec *)(meta[i] + 1);
> @@ -291,14 +261,11 @@ uint32_t get_num_cpus(void)
> xc_physinfo_t physinfo;
> int xc_handle = xc_interface_open();
> int ret;
> -
> +
> ret = xc_physinfo(xc_handle, &physinfo);
> -
> +
> if ( ret != 0 )
> - {
> - PERROR("Failure to get logical CPU count from Xen");
> - exit(EXIT_FAILURE);
> - }
> + errx(EXIT_FAILURE, "Failure to get logical CPU count from Xen");
>
> xc_interface_close(xc_handle);
>
> @@ -377,17 +344,17 @@ int parse_evtmask(char *arg, struct argp
> char *inval;
>
> /* search filtering class */
> - if (strcmp(arg, "gen") == 0){
> + if ( strcmp(arg, "gen") == 0 )
> setup->evt_mask |= TRC_GEN;
> - } else if(strcmp(arg, "sched") == 0){
> + else if ( strcmp(arg, "sched") == 0 )
> setup->evt_mask |= TRC_SCHED;
> - } else if(strcmp(arg, "dom0op") == 0){
> + else if ( strcmp(arg, "dom0op") == 0 )
> setup->evt_mask |= TRC_DOM0OP;
> - } else if(strcmp(arg, "vmx") == 0){
> + else if ( strcmp(arg, "vmx") == 0 )
> setup->evt_mask |= TRC_VMX;
> - } else if(strcmp(arg, "all") == 0){
> + else if ( strcmp(arg, "all") == 0 )
> setup->evt_mask |= TRC_ALL;
> - } else {
> + else {
> setup->evt_mask = strtol(arg, &inval, 0);
> if ( inval == arg )
> argp_usage(state);
> @@ -430,13 +397,13 @@ error_t cmd_parser(int key, char *arg, s
> argp_usage(state);
> }
> break;
> -
> +
> case 'e': /* set new event mask for filtering*/
> {
> parse_evtmask(arg, state);
> }
> break;
> -
> +
> case 'S': /* set tbuf size (given in pages) */
> {
> char *inval;
> @@ -454,7 +421,7 @@ error_t cmd_parser(int key, char *arg, s
> argp_usage(state);
> }
> break;
> -
> +
> default:
> return ARGP_ERR_UNKNOWN;
> }
> @@ -473,16 +440,16 @@ const struct argp_option cmd_opts[] =
> "(default " xstr(NEW_DATA_THRESH) ")." },
>
> { .name = "poll-sleep", .key='s', .arg="p",
> - .doc =
> + .doc =
> "Set sleep time, p, in milliseconds between polling the trace buffer
> " "for new data (default " xstr(POLL_SLEEP_MILLIS) ")." },
>
> { .name = "cpu-mask", .key='c', .arg="c",
> - .doc =
> + .doc =
> "set cpu-mask " },
>
> { .name = "evt-mask", .key='e', .arg="e",
> - .doc =
> + .doc =
> "set evt-mask " },
>
> { .name = "trace-buf-size", .key='S', .arg="N",
> @@ -504,7 +471,7 @@ const struct argp parser_def =
> "\v"
> "This tool is used to capture trace buffer data from Xen. The data is
> " "output in a binary format, in the following order:\n\n"
> - " CPU(uint) TSC(uint64_t) EVENT(uint32_t) D1 D2 D3 D4 D5 "
> + " CPU(uint32_t) TSC(uint64_t) EVENT(uint32_t) D1 D2 D3 D4 D5 "
> "(all uint32_t)\n\n"
> "The output should be parsed using the tool xentrace_format, which can
> " "produce human-readable output in ASCII format."
> @@ -513,8 +480,8 @@ const struct argp parser_def =
>
> const char *argp_program_version = "xentrace v1.1";
> const char *argp_program_bug_address = "<mark.a.williamson@xxxxxxxxx>";
> -
> -
> +
> +
> int main(int argc, char **argv)
> {
> int outfd = 1, ret;
> @@ -529,31 +496,23 @@ int main(int argc, char **argv)
>
> argp_parse(&parser_def, argc, argv, 0, 0, &opts);
>
> - if (opts.evt_mask != 0) {
> + if ( opts.evt_mask != 0 )
> set_mask(opts.evt_mask, 0);
> - }
>
> - if (opts.cpu_mask != 0) {
> + if ( opts.cpu_mask != 0 )
> set_mask(opts.cpu_mask, 1);
> - }
>
> if ( opts.outfile )
> outfd = open(opts.outfile, O_WRONLY | O_CREAT | O_LARGEFILE,
> 0644);
>
> - if(outfd < 0)
> - {
> - perror("Could not open output file");
> - exit(EXIT_FAILURE);
> - }
> + if ( outfd < 0 )
> + err(EXIT_FAILURE, "Could not open output file");
>
> - if(isatty(outfd))
> - {
> - fprintf(stderr, "Cannot output to a TTY, specify a log file.\n");
> - exit(EXIT_FAILURE);
> - }
> + if ( isatty(outfd) )
> + errx(EXIT_FAILURE, "Cannot output to a TTY, specify a log
> file.\n");
>
> logfile = fdopen(outfd, "w");
> -
> +
> /* ensure that if we get a signal, we'll do cleanup, then exit */
> act.sa_handler = close_handler;
> act.sa_flags = 0;
> Index: xen-unstable.hg-mainline.xentrace/tools/xentrace/xentrace_format
> ===================================================================
> --- xen-unstable.hg-mainline.xentrace.orig/tools/xentrace/xentrace_format
> +++ xen-unstable.hg-mainline.xentrace/tools/xentrace/xentrace_format
> @@ -24,7 +24,7 @@ def usage():
> Which correspond to the CPU number, event ID, timestamp counter
> and the 5 data fields from the trace record. There should be one such rule
> for each type of event.
> -
> +
> Depending on your system and the volume of trace buffer data,
> this script may not be able to keep up with the output of
> xentrace if it is piped directly. In these circumstances you should have
> @@ -34,7 +34,7 @@ def usage():
>
> def read_defs(defs_file):
> defs = {}
> -
> +
> fd = open(defs_file)
>
> reg = re.compile('(\S+)\s+(\S.*)')
> @@ -43,14 +43,14 @@ def read_defs(defs_file):
> line = fd.readline()
> if not line:
> break
> -
> - if line[0] == '#' or line[0] == '\n':
> - continue
> -
> +
> + if line[0] == '#' or line[0] == '\n':
> + continue
> +
> m = reg.match(line)
>
> if not m: print >> sys.stderr, "Bad format file" ; sys.exit(1)
> -
> +
> defs[str(eval(m.group(1)))] = m.group(2)
>
> return defs
> @@ -70,7 +70,7 @@ try:
> opts, arg = getopt.getopt(sys.argv[1:], "c:" )
>
> for opt in opts:
> - if opt[0] == '-c' : mhz = int(opt[1])
> + if opt[0] == '-c' : mhz = int(opt[1])
>
> except getopt.GetoptError:
> usage()
> @@ -96,7 +96,7 @@ i=0
>
> while not interrupted:
> try:
> - i=i+1
> + i=i+1
> line = sys.stdin.read(struct.calcsize(CPUREC))
> if not line:
> break
> @@ -108,19 +108,20 @@ while not interrupted:
>
> (tsc, event, d1, d2, d3, d4, d5) = struct.unpack(TRCREC, line)
>
> - #tsc = (tscH<<32) | tscL
> + #tsc = (tscH<<32) | tscL
>
> - #print i, tsc
> + #print i, tsc
>
> if cpu >= len(last_tsc):
> last_tsc += [0] * (cpu - len(last_tsc) + 1)
> - elif tsc < last_tsc[cpu]:
> - print "TSC stepped backward cpu %d ! %d %d" %
> (cpu,tsc,last_tsc[cpu]) + elif tsc < last_tsc[cpu]:
> + print "TSC stepped backward cpu %d ! %d %d" % (cpu, tsc,
> + last_tsc[cpu])
>
> - last_tsc[cpu] = tsc
> + last_tsc[cpu] = tsc
>
> - if mhz:
> - tsc = tsc / (mhz*1000000.0)
> + if mhz:
> + tsc = tsc / (mhz*1000000.0)
>
> args = {'cpu' : cpu,
> 'tsc' : tsc,
> @@ -131,15 +132,13 @@ while not interrupted:
> '4' : d4,
> '5' : d5 }
>
> - try:
> -
> - if defs.has_key(str(event)):
> - print defs[str(event)] % args
> - else:
> - if defs.has_key(str(0)): print defs[str(0)] % args
> - except TypeError:
> - print defs[str(event)]
> - print args
> -
> + try:
> + if defs.has_key(str(event)):
> + print defs[str(event)] % args
> + else:
> + if defs.has_key(str(0)): print defs[str(0)] % args
> + except TypeError:
> + print defs[str(event)]
> + print args
>
> except IOError, struct.error: sys.exit()
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-devel
--
Dave: Just a question. What use is a unicyle with no seat? And no pedals!
Mark: To answer a question with a question: What use is a skateboard?
Dave: Skateboards have wheels.
Mark: My wheel has a wheel!
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home