On Fri, May 22, 2015 at 05:44:47PM -0500, Charles Duffy wrote: > From: Charles Duffy <chadu...@cisco.com> > > --- > src/cgtop/cgtop.c | 64 > ++++++++++++++++++++++++++++++++++++++++++++----------- > 1 file changed, 51 insertions(+), 13 deletions(-) > > diff --git a/src/cgtop/cgtop.c b/src/cgtop/cgtop.c > index a390cf3..45c8d6f 100644 > --- a/src/cgtop/cgtop.c > +++ b/src/cgtop/cgtop.c > @@ -77,6 +77,15 @@ static enum { > CPU_TIME, > } arg_cpu_type = CPU_PERCENT; > > +enum ByteRepresentation { > + BYTES_HUMAN, > + BYTES_RAW, > +}; > +typedef enum ByteRepresentation ByteRepresentation; > + > +static ByteRepresentation arg_memory_type = BYTES_HUMAN; > +static ByteRepresentation arg_io_type = BYTES_HUMAN; > + > static void group_free(Group *g) { > assert(g); > > @@ -96,6 +105,16 @@ static void group_hashmap_free(Hashmap *h) { > hashmap_free(h); > } > > +static char *cond_format_bytes(char *buf, size_t l, off_t t, > ByteRepresentation r, bool is_valid) { This should return const char *.
> + if (!is_valid) > + return (char*)" -"; Width formatting is already done in the caller of this function. So just "-" should be enough. > + if (r == BYTES_HUMAN) > + return format_bytes(buf, l, t); > + snprintf(buf, l, "%lu8", t); What is "8" doing in the format? Also off_t is signed. I think "%zd" is the right format. > + buf[l-1] = 0; snprintf terminates with NUL on its own, always, so this line is not needed. Zbyszek > + return buf; > +} > + > static int process(const char *controller, const char *path, Hashmap *a, > Hashmap *b, unsigned iteration) { > Group *g; > int r; > @@ -532,18 +551,9 @@ static int display(Hashmap *a) { > } else > printf(" %*s", maxtcpu, format_timespan(buffer, > sizeof(buffer), (nsec_t) (g->cpu_usage / NSEC_PER_USEC), 0)); > > - if (g->memory_valid) > - printf(" %8s", format_bytes(buffer, sizeof(buffer), > g->memory)); > - else > - fputs(" -", stdout); > - > - if (g->io_valid) { > - printf(" %8s", > - format_bytes(buffer, sizeof(buffer), > g->io_input_bps)); > - printf(" %8s", > - format_bytes(buffer, sizeof(buffer), > g->io_output_bps)); > - } else > - fputs(" - -", stdout); > + printf(" %8s", cond_format_bytes(buffer, sizeof(buffer), > g->memory, arg_memory_type, g->memory_valid)); > + printf(" %8s", cond_format_bytes(buffer, sizeof(buffer), > g->io_input_bps, arg_io_type, g->io_valid)); > + printf(" %8s", cond_format_bytes(buffer, sizeof(buffer), > g->io_output_bps, arg_io_type, g->io_valid)); > > putchar('\n'); > } > @@ -562,6 +572,8 @@ static void help(void) { > " -m Order by memory load\n" > " -i Order by IO load\n" > " --cpu[=TYPE] Show CPU usage as time or percentage > (default)\n" > + " --memory[=TYPE] Show memory usage as bytes or human > (default)\n" > + " --io[=TYPE] Show memory usage as bytes or human > (default)\n" So, I know that Umut suggested this, but I'm not sure that this is a good idea. When would you want different units for memory and io? One switch should be enough. Zbyszek > " -d --delay=DELAY Delay between updates\n" > " -n --iterations=N Run for N iterations before exiting\n" > " -b --batch Run in batch mode, accepting no > input\n" > @@ -574,7 +586,9 @@ static int parse_argv(int argc, char *argv[]) { > enum { > ARG_VERSION = 0x100, > ARG_DEPTH, > - ARG_CPU_TYPE > + ARG_CPU_TYPE, > + ARG_MEM_TYPE, > + ARG_IO_TYPE, > }; > > static const struct option options[] = { > @@ -585,6 +599,8 @@ static int parse_argv(int argc, char *argv[]) { > { "batch", no_argument, NULL, 'b' }, > { "depth", required_argument, NULL, ARG_DEPTH }, > { "cpu", optional_argument, NULL, ARG_CPU_TYPE}, > + { "memory", optional_argument, NULL, ARG_MEM_TYPE}, > + { "io", optional_argument, NULL, ARG_IO_TYPE }, > {} > }; > > @@ -618,6 +634,28 @@ static int parse_argv(int argc, char *argv[]) { > } > break; > > + case ARG_MEM_TYPE: > + if (optarg) { > + if (strcmp(optarg, "human") == 0) > + arg_memory_type = BYTES_HUMAN; > + else if (strcmp(optarg, "bytes") == 0) > + arg_memory_type = BYTES_RAW; > + else > + return -EINVAL; > + } > + break; > + > + case ARG_IO_TYPE: > + if (optarg) { > + if (strcmp(optarg, "human") == 0) > + arg_io_type = BYTES_HUMAN; > + else if (strcmp(optarg, "bytes") == 0) > + arg_io_type = BYTES_RAW; > + else > + return -EINVAL; > + } > + break; > + > case ARG_DEPTH: > r = safe_atou(optarg, &arg_depth); > if (r < 0) { > -- > 2.0.0 > > _______________________________________________ > systemd-devel mailing list > systemd-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/systemd-devel _______________________________________________ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel