[systemd-devel] [PATCH] RFC: util: Avoid memory allocations for formatting paths

2013-04-02 Thread Holger Freyther 
From: Holger Hans Peter Freyther 

Avoid memory allocations to construct the path for files in the
procfs. The procfs paths are way shorter than the PATH_MAX so we
can use snprintf on a string located on the stack. This shows up
as a win on x86 using the benchmark program below.

$ make libsystemd-shared.la; gcc -O2 -Isrc/systemd/ -Isrc/ \
-o simple-perf-test simple-perf-test.c \
.libs/libsystemd-shared.a  -lrt

 #include "shared/util.h"
void test_once(void) {
pid_t pid = getpid();
char *tmp = NULL;

get_process_comm(pid, &tmp);
free(tmp);
tmp = NULL;
get_process_cmdline(pid, 0, 1, &tmp);
free(tmp);
is_kernel_thread(pid);
tmp = NULL;
get_process_exe(pid, &tmp);
free(tmp);
}

int main(int argc, char **argv)
{
int i;
for (i = 0; i < 5; ++i)
test_once();
}
---
 src/shared/util.c |   51 ++-
 1 file changed, 22 insertions(+), 29 deletions(-)

diff --git a/src/shared/util.c b/src/shared/util.c
index d861ca9..65b546d 100644
--- a/src/shared/util.c
+++ b/src/shared/util.c
@@ -80,6 +80,13 @@ char **saved_argv = NULL;
 static volatile unsigned cached_columns = 0;
 static volatile unsigned cached_lines = 0;
 
+#define PROCFS_PATH_LEN (sizeof("/proc/")-1 + DECIMAL_STR_MAX(unsigned long))
+
+#define format_procfs_path(buffer, path, pid) do { \
+snprintf(buffer, sizeof(buffer) - 1, "/proc/%lu/%s", (unsigned long) 
pid, path); \
+char_array_0(buffer); } while(0)
+
+
 size_t page_size(void) {
 static __thread size_t pgsz = 0;
 long r;
@@ -571,12 +578,9 @@ int get_process_comm(pid_t pid, char **name) {
 if (pid == 0)
 r = read_one_line_file("/proc/self/comm", name);
 else {
-char *p;
-if (asprintf(&p, "/proc/%lu/comm", (unsigned long) pid) < 0)
-return -ENOMEM;
-
-r = read_one_line_file(p, name);
-free(p);
+char path[PROCFS_PATH_LEN + sizeof("/comm")];
+format_procfs_path(path, "comm", pid);
+r = read_one_line_file(path, name);
 }
 
 return r;
@@ -592,12 +596,9 @@ int get_process_cmdline(pid_t pid, size_t max_length, bool 
comm_fallback, char *
 if (pid == 0)
 f = fopen("/proc/self/cmdline", "re");
 else {
-char *p;
-if (asprintf(&p, "/proc/%lu/cmdline", (unsigned long) pid) < 0)
-return -ENOMEM;
-
-f = fopen(p, "re");
-free(p);
+char path[PROCFS_PATH_LEN + sizeof("/cmdline")];
+format_procfs_path(path, "cmdline", pid);
+f = fopen(path, "re");
 }
 
 if (!f)
@@ -684,7 +685,7 @@ int get_process_cmdline(pid_t pid, size_t max_length, bool 
comm_fallback, char *
 }
 
 int is_kernel_thread(pid_t pid) {
-char *p;
+char path[PROCFS_PATH_LEN + sizeof("/cmdline")];
 size_t count;
 char c;
 bool eof;
@@ -693,11 +694,8 @@ int is_kernel_thread(pid_t pid) {
 if (pid == 0)
 return 0;
 
-if (asprintf(&p, "/proc/%lu/cmdline", (unsigned long) pid) < 0)
-return -ENOMEM;
-
-f = fopen(p, "re");
-free(p);
+format_procfs_path(path, "cmdline", pid);
+f = fopen(path, "re");
 
 if (!f)
 return -errno;
@@ -722,12 +720,9 @@ int get_process_exe(pid_t pid, char **name) {
 if (pid == 0)
 r = readlink_malloc("/proc/self/exe", name);
 else {
-char *p;
-if (asprintf(&p, "/proc/%lu/exe", (unsigned long) pid) < 0)
-return -ENOMEM;
-
-r = readlink_malloc(p, name);
-free(p);
+char path[PROCFS_PATH_LEN + sizeof("/exe")];
+format_procfs_path(path, "exe", pid);
+r = readlink_malloc(path, name);
 }
 
 return r;
@@ -735,7 +730,7 @@ int get_process_exe(pid_t pid, char **name) {
 
 static int get_process_id(pid_t pid, const char *field, uid_t *uid) {
 _cleanup_fclose_ FILE *f = NULL;
-_cleanup_free_ char *p = NULL;
+char path[PROCFS_PATH_LEN + sizeof("/status")];
 char line[LINE_MAX];
 
 assert(field);
@@ -744,10 +739,8 @@ static int get_process_id(pid_t pid, const char *field, 
uid_t *uid) {
 if (pid == 0)
 return getuid();
 
-if (asprintf(&p, "/proc/%lu/status", (unsigned long) pid) < 0)
-return -ENOMEM;
-
-f = fopen(p, "re");
+format_procfs_path(path, "status", pid);
+f = fopen(path, "re");
 if (!f)
 return -errno;
 
-- 
1.7.10.4

___
systemd-devel mailing list
systemd-devel@lists.freedesk

Re: [systemd-devel] [PATCH 1/2] RFC: util: Avoid memory allocations for formatting paths

2013-04-02 Thread Zbigniew Jędrzejewski-Szmek 
On Tue, Apr 02, 2013 at 10:35:50PM +0200, Holger Freyther wrote:
> +#define format_procfs_path(buffer, path, pid) \
> +snprintf(buffer, sizeof(buffer) - 1, "/proc/%lu/%s", (unsigned long) 
> pid, path); \
> +char_array_0(buffer);
> +
Hi,
Lennart recently implemented a smarter way to do those short printfs:
http://cgit.freedesktop.org/systemd/systemd/commit/?id=fa70beaabc56762fdf77e675c3e09bb638d89938.
No need to use PATH_MAX.

Zbyszek
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH 1/2] RFC: util: Avoid memory allocations for formatting paths

2013-04-02 Thread Holger Freyther 
Ian Pilcher  gmail.com> writes:


> http://gcc.gnu.org/onlinedocs/cpp/Swallowing-the-Semicolon.html

thanks, good point. I will add a do {} while(0) around it.




___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH 1/2] RFC: util: Avoid memory allocations for formatting paths

2013-04-02 Thread Ian Pilcher 
On 04/02/2013 03:35 PM, Holger Freyther wrote:
> +#define format_procfs_path(buffer, path, pid) \
> +snprintf(buffer, sizeof(buffer) - 1, "/proc/%lu/%s", (unsigned long) 
> pid, path); \
> +char_array_0(buffer);

http://gcc.gnu.org/onlinedocs/cpp/Swallowing-the-Semicolon.html

-- 

Ian Pilcher arequip...@gmail.com
Sometimes there's nothing left to do but crash and burn...or die trying.


___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH 1/2] RFC: util: Avoid memory allocations for formatting paths

2013-04-02 Thread Holger Freyther 
From: Holger Hans Peter Freyther 

Avoid memory allocations to construct the path for files in the
procfs. The procfs paths are way shorter than the PATH_MAX so we
can use snprintf on a string located on the stack. This shows up
as a win on x86 using the benchmark program below.

$ make libsystemd-shared.la; gcc -O2 -Isrc/systemd/ -Isrc/ \
-o simple-perf-test simple-perf-test.c \
.libs/libsystemd-shared.a  -lrt

 #include "shared/util.h"
void test_once(void) {
pid_t pid = getpid();
char *tmp = NULL;

get_process_comm(pid, &tmp);
free(tmp);
tmp = NULL;
get_process_cmdline(pid, 0, 1, &tmp);
free(tmp);
is_kernel_thread(pid);
tmp = NULL;
get_process_exe(pid, &tmp);
free(tmp);
}

int main(int argc, char **argv)
{
int i;
for (i = 0; i < 5; ++i)
test_once();
}
---
 src/shared/util.c |   48 +++-
 1 file changed, 19 insertions(+), 29 deletions(-)

diff --git a/src/shared/util.c b/src/shared/util.c
index b516b9b..ce8f866 100644
--- a/src/shared/util.c
+++ b/src/shared/util.c
@@ -567,6 +567,10 @@ char *truncate_nl(char *s) {
 return s;
 }
 
+#define format_procfs_path(buffer, path, pid) \
+snprintf(buffer, sizeof(buffer) - 1, "/proc/%lu/%s", (unsigned long) 
pid, path); \
+char_array_0(buffer);
+
 int get_process_comm(pid_t pid, char **name) {
 int r;
 
@@ -575,12 +579,9 @@ int get_process_comm(pid_t pid, char **name) {
 if (pid == 0)
 r = read_one_line_file("/proc/self/comm", name);
 else {
-char *p;
-if (asprintf(&p, "/proc/%lu/comm", (unsigned long) pid) < 0)
-return -ENOMEM;
-
-r = read_one_line_file(p, name);
-free(p);
+char path[PATH_MAX];
+format_procfs_path(path, "comm", pid);
+r = read_one_line_file(path, name);
 }
 
 return r;
@@ -596,12 +597,9 @@ int get_process_cmdline(pid_t pid, size_t max_length, bool 
comm_fallback, char *
 if (pid == 0)
 f = fopen("/proc/self/cmdline", "re");
 else {
-char *p;
-if (asprintf(&p, "/proc/%lu/cmdline", (unsigned long) pid) < 0)
-return -ENOMEM;
-
-f = fopen(p, "re");
-free(p);
+char path[PATH_MAX];
+format_procfs_path(path, "cmdline", pid);
+f = fopen(path, "re");
 }
 
 if (!f)
@@ -688,7 +686,7 @@ int get_process_cmdline(pid_t pid, size_t max_length, bool 
comm_fallback, char *
 }
 
 int is_kernel_thread(pid_t pid) {
-char *p;
+char path[PATH_MAX];
 size_t count;
 char c;
 bool eof;
@@ -697,11 +695,8 @@ int is_kernel_thread(pid_t pid) {
 if (pid == 0)
 return 0;
 
-if (asprintf(&p, "/proc/%lu/cmdline", (unsigned long) pid) < 0)
-return -ENOMEM;
-
-f = fopen(p, "re");
-free(p);
+format_procfs_path(path, "cmdline", pid);
+f = fopen(path, "re");
 
 if (!f)
 return -errno;
@@ -726,12 +721,9 @@ int get_process_exe(pid_t pid, char **name) {
 if (pid == 0)
 r = readlink_malloc("/proc/self/exe", name);
 else {
-char *p;
-if (asprintf(&p, "/proc/%lu/exe", (unsigned long) pid) < 0)
-return -ENOMEM;
-
-r = readlink_malloc(p, name);
-free(p);
+char path[PATH_MAX];
+format_procfs_path(path, "exe", pid);
+r = readlink_malloc(path, name);
 }
 
 return r;
@@ -739,7 +731,7 @@ int get_process_exe(pid_t pid, char **name) {
 
 static int get_process_id(pid_t pid, const char *field, uid_t *uid) {
 _cleanup_fclose_ FILE *f = NULL;
-_cleanup_free_ char *p = NULL;
+char path[PATH_MAX];
 char line[LINE_MAX];
 
 assert(field);
@@ -748,10 +740,8 @@ static int get_process_id(pid_t pid, const char *field, 
uid_t *uid) {
 if (pid == 0)
 return getuid();
 
-if (asprintf(&p, "/proc/%lu/status", (unsigned long) pid) < 0)
-return -ENOMEM;
-
-f = fopen(p, "re");
+format_procfs_path(path, "status", pid);
+f = fopen(path, "re");
 if (!f)
 return -errno;
 
-- 
1.7.10.4

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH 2/2] RFC: util: Avoid doing work on the string when it is not used

2013-04-02 Thread Holger Freyther 
From: Holger Hans Peter Freyther 

The routine is dominated by the fgets and not the code that is
moved around. Doing the work only for the lines that will be
used should be a small gain. Routines in the selinux-access.c
should probably query the gid and uid at the same time instead
of opening and parsing the file twice.
---
 src/shared/util.c |6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/src/shared/util.c b/src/shared/util.c
index ce8f866..d5ed615 100644
--- a/src/shared/util.c
+++ b/src/shared/util.c
@@ -746,11 +746,9 @@ static int get_process_id(pid_t pid, const char *field, 
uid_t *uid) {
 return -errno;
 
 FOREACH_LINE(line, f, return -errno) {
-char *l;
 
-l = strstrip(line);
-
-if (startswith(l, field)) {
+if (startswith(line, field)) {
+char *l = strstrip(line);
 l += strlen(field);
 l += strspn(l, WHITESPACE);
 
-- 
1.7.10.4

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] New Qt wrapper: libsystemd-qt

2013-04-02 Thread Andrea Scarpino 
Hi all,
I'm new to this mailing list, but I want to introduce a new Qt library for the 
systemd API: libsystemd-qt[1].

One year and half ago another developer, Stefan Majewsky, announced the same 
work[2], but since then there has been no development of that library[3].
That library never built for me and I don't understand those python scripts 
that generate the cpp interfaces files.

Moreover, the new display manager SDDM, to which I contribute, will require 
some systemd integration (specially with logind). Also, Chakra has some plan 
for systemd integration in KDE.
That's because I started a new project.

The library ATM just list, start/stop, enable/disable the units (this is just 
what I did in few hours today :). The plain is to cover all systemd API.

I'm new, so I hope to get many feedbacks ;)

Cheers.

[1] https://github.com/ndr/libsystemd-qt
[2] http://lists.freedesktop.org/archives/systemd-devel/2011-August/003136.html
[3] http://quickgit.kde.org/?p=libqsystemd.git

-- 
Andrea
Arch Linux Developer
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] cgtop: Add cpu time opt and become stdout sensitive

2013-04-02 Thread Zbigniew Jędrzejewski-Szmek 
On Tue, Apr 02, 2013 at 06:52:16PM +0200, Umut Tezduyar wrote:
> ---
>  src/cgtop/cgtop.c |  122 
> ++---
>  1 files changed, 88 insertions(+), 34 deletions(-)
Hi Umut,
patch is applied. I removed the header when !on_tty() - I don't think
it has any use in that case. Should make your usecase even easier.

Zbyszek
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [HEADSUP] New module ‘libc’ in systemd git

2013-04-02 Thread Robert Schwebel 
On Tue, Apr 02, 2013 at 02:26:14PM +0200, Lennart Poettering wrote:
> > I just added a new module to systemd git, “libc”, and you are probably
> > wondering what that is about. Here’s a quick overview over what this is
> > and our plans with it.
>
> For the sake of the archives, and since this apparently wasn't obvious
> to everybody: this was our attempt to being funny on April 1st.
>
> There is no truth in the original mail.

Be careful - we will point you to this mail in the future 8-)

rsc
-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] [RFCv4] Add Listen* to dbus properties

2013-04-02 Thread Kok, Auke-jan H 
On Tue, Apr 2, 2013 at 6:29 AM, Zbigniew Jędrzejewski-Szmek
 wrote:
> On Tue, Apr 02, 2013 at 08:26:17AM +0200, Tomasz Torcz wrote:
>> On Tue, Apr 02, 2013 at 05:54:32AM +0200, Zbigniew Jędrzejewski-Szmek wrote:
>> > On Mon, Apr 01, 2013 at 11:09:45PM +0300, Oleksii Shevchuk wrote:
>> > > sockets.socket - Test
>> > > Loaded: loaded (/home/alxchk/.config/systemd/user/sockets.socket; 
>> > > static)
>> > > Active: inactive (dead)
>> > > Listen: Stream: /tmp/stream1
>> > > Stream: @stream4
>> > Pushed, but format changed to:
>> >
>> > listen1.socket - descr descr
>> >  Loaded: loaded (/run/systemd/system/listen1.socket; 
>> > static)
>> >  Active: failed (Result: resources)
>> >ListenStream: /tmp/stream1
>> >  ListenDatagram: /tmp/stream2
>>
>>   I must say I liked per-type grouping better.
> It *looked* better, but the ordering of sockets is important: it determines
> the order of fd's for the .service. For some applications it might not matter,
> but for simple ones it might, so it's better to preserve this information.

If ordering is important, the output should reflect the ordering. The
only way I can see that you could accomplish that unambiguously is by
numbering them. Sorting them will just leave people puzzled as to what
the order is.

So, consider adding some form of numbering to the list of listen
addresses. Perhaps something like:

>> >0: ListenStream: /tmp/stream1
>> >1: ListenDatagram: /tmp/stream2

Cheers,

Auke
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] cgtop: Add cpu time opt and become stdout sensitive

2013-04-02 Thread Umut Tezduyar 
Hi,

I have resubmitted the patch with agreed changes.

My usecase is pretty simple actually. I would like to store process time of
services right after default.target and store the values in a database.
That involves running cgtop 1 time and parsing the output.

Thanks.


On Mon, Apr 1, 2013 at 2:23 PM, Zbigniew Jędrzejewski-Szmek <
zbys...@in.waw.pl> wrote:

> On Mon, Apr 01, 2013 at 10:01:40AM +0200, Umut Tezduyar wrote:
> > Hi Zbigniew,
> >
> > Thank you very much for reviewing. Please find my comments below.
> >
> >
> > On Mon, Apr 1, 2013 at 6:19 AM, Zbigniew Jędrzejewski-Szmek <
> > zbys...@in.waw.pl> wrote:
> >
> > > On Sun, Mar 31, 2013 at 08:22:05PM +0200, Umut Tezduyar wrote:
> > > > ---
> > > >  src/cgtop/cgtop.c |  108
> > > -
> > > >  1 files changed, 74 insertions(+), 34 deletions(-)
> > > >
> > > > diff --git a/src/cgtop/cgtop.c b/src/cgtop/cgtop.c
> > > > index eebebf0..0ef4fcc 100644
> > > > --- a/src/cgtop/cgtop.c
> > > > +++ b/src/cgtop/cgtop.c
> > > > @@ -69,6 +69,11 @@ static enum {
> > > >  ORDER_IO
> > > >  } arg_order = ORDER_CPU;
> > > >
> > > > +static enum {
> > > > +CPU_PERCENT,
> > > > +CPU_TIME,
> > > > +} arg_cpu_type = CPU_PERCENT;
> > > > +
> > > >  static void group_free(Group *g) {
> > > >  assert(g);
> > > >
> > > > @@ -372,16 +377,22 @@ static int group_compare(const void*a, const
> void
> > > *b) {
> > > >  return 1;
> > > >
> > > >  if (arg_order == ORDER_CPU) {
> > > > -if (x->cpu_valid && y->cpu_valid) {
> > > > -
> > > > -if (x->cpu_fraction > y->cpu_fraction)
> > > > +if (arg_cpu_type == CPU_PERCENT) {
> > > > +if (x->cpu_valid && y->cpu_valid) {
> > > > +if (x->cpu_fraction >
> y->cpu_fraction)
> > > > +return -1;
> > > > +else if (x->cpu_fraction <
> > > y->cpu_fraction)
> > > > +return 1;
> > > > +} else if (x->cpu_valid)
> > > >  return -1;
> > > > -else if (x->cpu_fraction < y->cpu_fraction)
> > > > +else if (y->cpu_valid)
> > > >  return 1;
> > > > -} else if (x->cpu_valid)
> > > > -return -1;
> > > > -else if (y->cpu_valid)
> > > > -return 1;
> > > > +} else {
> > > > +if (x->cpu_usage > y->cpu_usage)
> > > > +return -1;
> > > > +else if (x->cpu_usage < y->cpu_usage)
> > > > +return 1;
> > > > +}
> > > >  }
> > > >
> > > >  if (arg_order == ORDER_TASKS) {
> > > > @@ -428,13 +439,16 @@ static int display(Hashmap *a) {
> > > >  Iterator i;
> > > >  Group *g;
> > > >  Group **array;
> > > > -unsigned rows, path_columns, n = 0, j;
> > > > +signed path_columns;
> > > > +unsigned rows, n = 0, j, maxtcpu = 0, maxtpath = 0;
> > > > +char cpu_title[21];
> > > >
> > > >  assert(a);
> > > >
> > > >  /* Set cursor to top left corner and clear screen */
> > > > -fputs("\033[H"
> > > > -  "\033[2J", stdout);
> > > > +if (on_tty())
> > > > +fputs("\033[H"
> > > > +  "\033[2J", stdout);
> > > >
> > > >  array = alloca(sizeof(Group*) * hashmap_size(a));
> > > >
> > > > @@ -444,33 +458,50 @@ static int display(Hashmap *a) {
> > > >
> > > >  qsort(array, n, sizeof(Group*), group_compare);
> > > >
> > > > +/* Find longest names in one run */
> > > > +for (j = 0; j < n; j++) {
> > > > +snprintf(cpu_title, sizeof(cpu_title), "%llu",
> > > array[j]->cpu_usage);
> > > > +if (strlen(cpu_title) > maxtcpu)
> > > > +maxtcpu = strlen(cpu_title);
> > > > +if (strlen(array[j]->path) > maxtpath)
> > > > +maxtpath = strlen(array[j]->path);
> > > The compiler *might* be smart enough, but let's not depend on that.
> > > strlen() result should be cached. Maybe just use MAX?
> > >
> > Umut: Sounds reasonable.
> >
> > >
> > > > +}
> > > > +
> > > > +if (arg_cpu_type == CPU_PERCENT)
> > > > +snprintf(cpu_title, sizeof(cpu_title), "%6s",
> "%CPU");
> > > > +else
> > > > +snprintf(cpu_title, sizeof(cpu_title), "%*s",
> maxtcpu,
> > > "CPU Time");
> > > > +
> > > >  rows = lines();
> > > >  if (rows <= 10)
> > > >  rows = 10;
> > > >
> > > > -path_columns = columns() - 42;
> > > > -if (path_columns < 10)
> > > > -path_columns

[systemd-devel] [PATCH] cgtop: Add cpu time opt and become stdout sensitive

2013-04-02 Thread Umut Tezduyar 
---
 src/cgtop/cgtop.c |  122 ++---
 1 files changed, 88 insertions(+), 34 deletions(-)

diff --git a/src/cgtop/cgtop.c b/src/cgtop/cgtop.c
index eebebf0..f124826 100644
--- a/src/cgtop/cgtop.c
+++ b/src/cgtop/cgtop.c
@@ -19,9 +19,11 @@
   along with systemd; If not, see .
 ***/
 
+#define __STDC_FORMAT_MACROS
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -69,6 +71,11 @@ static enum {
 ORDER_IO
 } arg_order = ORDER_CPU;
 
+static enum {
+CPU_PERCENT,
+CPU_TIME,
+} arg_cpu_type = CPU_PERCENT;
+
 static void group_free(Group *g) {
 assert(g);
 
@@ -372,16 +379,22 @@ static int group_compare(const void*a, const void *b) {
 return 1;
 
 if (arg_order == ORDER_CPU) {
-if (x->cpu_valid && y->cpu_valid) {
-
-if (x->cpu_fraction > y->cpu_fraction)
+if (arg_cpu_type == CPU_PERCENT) {
+if (x->cpu_valid && y->cpu_valid) {
+if (x->cpu_fraction > y->cpu_fraction)
+return -1;
+else if (x->cpu_fraction < y->cpu_fraction)
+return 1;
+} else if (x->cpu_valid)
 return -1;
-else if (x->cpu_fraction < y->cpu_fraction)
+else if (y->cpu_valid)
 return 1;
-} else if (x->cpu_valid)
-return -1;
-else if (y->cpu_valid)
-return 1;
+} else {
+if (x->cpu_usage > y->cpu_usage)
+return -1;
+else if (x->cpu_usage < y->cpu_usage)
+return 1;
+}
 }
 
 if (arg_order == ORDER_TASKS) {
@@ -428,13 +441,18 @@ static int display(Hashmap *a) {
 Iterator i;
 Group *g;
 Group **array;
-unsigned rows, path_columns, n = 0, j;
+signed path_columns;
+unsigned rows, n = 0, j, maxtcpu = 0, maxtpath = 0;
+char cpu_title[21];
+const char *on = on_tty() ? ANSI_HIGHLIGHT_ON : "";
+const char *off = on_tty() ? ANSI_HIGHLIGHT_OFF : "";
 
 assert(a);
 
 /* Set cursor to top left corner and clear screen */
-fputs("\033[H"
-  "\033[2J", stdout);
+if (on_tty())
+fputs("\033[H"
+  "\033[2J", stdout);
 
 array = alloca(sizeof(Group*) * hashmap_size(a));
 
@@ -444,33 +462,51 @@ static int display(Hashmap *a) {
 
 qsort(array, n, sizeof(Group*), group_compare);
 
+/* Find the longest names in one run */
+for (j = 0; j < n; j++) {
+unsigned cputlen, pathtlen;
+snprintf(cpu_title, sizeof(cpu_title), "%"PRIu64, 
array[j]->cpu_usage);
+cputlen = strlen(cpu_title);
+maxtcpu = MAX(maxtcpu, cputlen);
+pathtlen = strlen(array[j]->path);
+maxtpath = MAX(maxtpath, pathtlen);
+}
+
+if (arg_cpu_type == CPU_PERCENT)
+snprintf(cpu_title, sizeof(cpu_title), "%6s", "%CPU");
+else
+snprintf(cpu_title, sizeof(cpu_title), "%*s", maxtcpu, "CPU 
Time");
+
 rows = lines();
 if (rows <= 10)
 rows = 10;
 
-path_columns = columns() - 42;
-if (path_columns < 10)
-path_columns = 10;
-
-printf("%s%-*s%s %s%7s%s %s%6s%s %s%8s%s %s%8s%s %s%8s%s\n\n",
-   arg_order == ORDER_PATH   ? ANSI_HIGHLIGHT_ON : "", 
path_columns, "Path",
-  arg_order == ORDER_PATH   ? ANSI_HIGHLIGHT_OFF : "",
-   arg_order == ORDER_TASKS  ? ANSI_HIGHLIGHT_ON : "", "Tasks",
-  arg_order == ORDER_TASKS  ? ANSI_HIGHLIGHT_OFF : "",
-   arg_order == ORDER_CPU? ANSI_HIGHLIGHT_ON : "", "%CPU",
-  arg_order == ORDER_CPU? ANSI_HIGHLIGHT_OFF : "",
-   arg_order == ORDER_MEMORY ? ANSI_HIGHLIGHT_ON : "", "Memory",
-  arg_order == ORDER_MEMORY ? ANSI_HIGHLIGHT_OFF : "",
-   arg_order == ORDER_IO ? ANSI_HIGHLIGHT_ON : "", "Input/s",
-  arg_order == ORDER_IO ? ANSI_HIGHLIGHT_OFF : "",
-   arg_order == ORDER_IO ? ANSI_HIGHLIGHT_ON : "", "Output/s",
-  arg_order == ORDER_IO ? ANSI_HIGHLIGHT_OFF : "");
+if (on_tty()) {
+path_columns = columns() - 36 - strlen(cpu_title);
+if (path_columns < 10)
+path_columns = 10;
+} else
+path_columns = maxtpath;
+
+printf("%s%-*s%s %s%7s%s %s%s%s %s

Re: [systemd-devel] [HEADSUP] New module ‘libc’ in systemd git

2013-04-02 Thread Lennart Poettering 
On Tue, 02.04.13 15:20, Colin Guthrie (gm...@colin.guthr.ie) wrote:

> 'Twas brillig, and Lennart Poettering at 02/04/13 13:26 did gyre and gimble:
> > On Mon, 01.04.13 02:22, Lennart Poettering (lenn...@poettering.net) wrote:
> > 
> >> Heya,
> >>
> >> I just added a new module to systemd git, “libc”, and you are probably
> >> wondering what that is about. Here’s a quick overview over what this is
> >> and our plans with it.
> > 
> > For the sake of the archives, and since this apparently wasn't obvious
> > to everybody: this was our attempt to being funny on April 1st.
> > 
> > There is no truth in the original mail.
> 
> Really? There were people who didn't get it? I saw a few random comments
> in various places but it seems everyone got the joke (with the exception
> of /. where no one really commented on the article but rather bitched
> about how all the text on /. was presented in rot13... I expect nothing
> less from that particular peanut gallery).
> 
> :)

Well, I got flamed personally about it via IM and IRC /msg, and people
have been asking on #systemd where to find the sources...

Surprisingly many people didn't get it...

Lennart

-- 
Lennart Poettering - Red Hat, Inc.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [HEADSUP] New module ‘libc’ in systemd git

2013-04-02 Thread Colin Guthrie 
'Twas brillig, and Lennart Poettering at 02/04/13 13:26 did gyre and gimble:
> On Mon, 01.04.13 02:22, Lennart Poettering (lenn...@poettering.net) wrote:
> 
>> Heya,
>>
>> I just added a new module to systemd git, “libc”, and you are probably
>> wondering what that is about. Here’s a quick overview over what this is
>> and our plans with it.
> 
> For the sake of the archives, and since this apparently wasn't obvious
> to everybody: this was our attempt to being funny on April 1st.
> 
> There is no truth in the original mail.

Really? There were people who didn't get it? I saw a few random comments
in various places but it seems everyone got the joke (with the exception
of /. where no one really commented on the article but rather bitched
about how all the text on /. was presented in rot13... I expect nothing
less from that particular peanut gallery).

:)

Col

-- 

Colin Guthrie
gmane(at)colin.guthr.ie
http://colin.guthr.ie/

Day Job:
  Tribalogic Limited http://www.tribalogic.net/
Open Source:
  Mageia Contributor http://www.mageia.org/
  PulseAudio Hacker http://www.pulseaudio.org/
  Trac Hacker http://trac.edgewall.org/

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] [RFCv4] Add Listen* to dbus properties

2013-04-02 Thread Zbigniew Jędrzejewski-Szmek 
On Tue, Apr 02, 2013 at 08:26:17AM +0200, Tomasz Torcz wrote:
> On Tue, Apr 02, 2013 at 05:54:32AM +0200, Zbigniew Jędrzejewski-Szmek wrote:
> > On Mon, Apr 01, 2013 at 11:09:45PM +0300, Oleksii Shevchuk wrote:
> > > sockets.socket - Test
> > > Loaded: loaded (/home/alxchk/.config/systemd/user/sockets.socket; 
> > > static)
> > > Active: inactive (dead)
> > > Listen: Stream: /tmp/stream1
> > > Stream: @stream4
> > Pushed, but format changed to:
> > 
> > listen1.socket - descr descr
> >  Loaded: loaded (/run/systemd/system/listen1.socket; static)
> >  Active: failed (Result: resources)
> >ListenStream: /tmp/stream1
> >  ListenDatagram: /tmp/stream2
> 
>   I must say I liked per-type grouping better.
It *looked* better, but the ordering of sockets is important: it determines
the order of fd's for the .service. For some applications it might not matter,
but for simple ones it might, so it's better to preserve this information.

Zbyszek
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [HEADSUP] New module ‘libc’ in systemd git

2013-04-02 Thread Lennart Poettering 
On Mon, 01.04.13 02:22, Lennart Poettering (lenn...@poettering.net) wrote:

> Heya,
> 
> I just added a new module to systemd git, “libc”, and you are probably
> wondering what that is about. Here’s a quick overview over what this is
> and our plans with it.

For the sake of the archives, and since this apparently wasn't obvious
to everybody: this was our attempt to being funny on April 1st.

There is no truth in the original mail.

Lennart

-- 
Lennart Poettering - Red Hat, Inc.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Duplicate assignment of kernel/sysrq

2013-04-02 Thread Harald Hoyer 
Am 02.04.2013 01:53, schrieb Reindl Harald:
> 
> 
> Am 02.04.2013 01:28, schrieb Tom Gundersen:
>> That said, the fact that we are currently parsing /etc/sysctl.conf is
>> not documented
> 
> and you broke it completly in Fedora 18 which is why the serivice
> below exists to get "net.ipv4.ip_forward = 1" of sysctl.conf
> assigend and let my routers still work as routers as also the
> service is prepeared on any F17 machine to not break with the
> Fedora 18 update
> 
> there are things which should not be touched for fun
> sysctl.conf is one of them!
> 
> 
> [root@srv-rhsoft:~]$ cat /etc/systemd/system/sysctl-post-network.service
> [Unit]
> Description=apply sysctl.conf after network
> After=network.target network.service openvpn.service
> 
> [Service]
> Type=oneshot
> ExecStart=/usr/sbin/sysctl -p
> RemainAfterExit=yes
> StandardOutput=null
> 
> [Install]
> WantedBy=multi-user.target
> 
> 

Which points us to a design flaw. Ideally the values would be applied as soon as
the files show up in /proc/sys. Something like udev for /proc/sys :)

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Shared root fs by default

2013-04-02 Thread Tvrtko Ursulin 

Hi,

On Friday 29 March 2013 15:56:01 Lennart Poettering wrote:
> On Thu, 28.03.13 16:47, Tvrtko Ursulin (tvrtko.ursu...@onelan.co.uk) wrote:
> > Hi all,
> > 
> > As a bit of a feedback, the change in systemd to mark root fs as
> > recursively shared by default has the potential to bite hard anyone who
> > builds chroot-ed environments on their system.
> > 
> > When you build your chroot fs and then bind mount bits of the outside
> > world
> > into it, you are up for a nasty surprise when you tear those mounts down.
> > You will then find your original mount points gone and system potentially
> > in a seriously broken state.
> 
> Well, not really. chroot()s and mount propagation are orthogonal, so we
> didn't really change much there. i.e. mounts you do from within chroots
> will also show up in the host (though shifted by the chroot's root dir
> of course), and if you drop them in the chroot they will disappear in
> the host too (also shifted by the chroot's root dir). So nothing really
> changed here.

I wasn't talking about this, perhaps I wasn't clear enough. See below.
 
> > Depending what bits from the outside have been bind mounted into chroot
> > and
> > your filesystem setup you can lose /home, /dev, /proc, etc. You get the
> > picture. :/
> 
> Nah, not true. You cannot umount the host's /home, /dev, /proc from
> inside the chroot, since you cannot even "see" them, and if you did bind
> mount them, then you will lose only the bind mounts, not the
> originals.
> 
> The only place where things change is that if you bind mounted /home
> into the chroot's root dir's /home, and then create a another submount
> below that and assumed it wouldn't also affect the hosts's original
> /home.

You need to try it since you don't seem to believe me. :)

+ M1=testmp1
+ M2=testmp2
+ SM=submount
+ mkdir -p testmp1
+ mkdir -p testmp2
+ mount none -t tmpfs testmp1
+ mkdir -p testmp1/submount
+ mount none -t tmpfs testmp1/submount
+ strace -f -e trace=mount,umount mount testmp1 --rbind testmp2
mount("/root/3/testmp1", "testmp2", 0x7f70e796a67c, MS_MGC_VAL|MS_BIND|MS_REC, 
NULL) = 0
+ strace -f -e trace=mount,umount umount -l testmp2
umount("/root/3/testmp2", MNT_DETACH)   = 0
+ mountpoint testmp1/submount
testmp1/submount is not a mountpoint

So now just imagine "testmp1/submount" was "/dev/pts".

Regards,

Tvrtko

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] /var/log/journal/

2013-04-02 Thread Umut Tezduyar 
Hi,

I am seeing the same behavior since I switched from 197 to 198. I have the
default journal.conf and all my logs go under /var/log/journal (My /var is
tmpfs) where before they were going under /run/log/journal. I haven't
investigated it but maybe the default configuration for Storage= is now
persistent and we haven't updated the journal.conf and it still says
#Storage=auto.
Hope helps a bit.

Good luck.


On Tue, Apr 2, 2013 at 2:21 AM, Reindl Harald wrote:

> was "/var/log/journal" not the folder you had to create
> for persistent storage if you want completly  switch
> to journald?
>
> systemd-200-3.fc19.x86_64
>
> i have for sure removed the folder some days ago on this rawhide-vm
> journald is a nice idea but not the way to go in every environemt
> so please be careful with such changes
> ___
>
> [5.527695] systemd-journald[142]: File
> /var/log/journal/74e76163c28448fba68b8667eb7b5d92/system.journal
> corrupted or uncleanly shut down, renaming and replacing.
> [root@rawhide ~]# ls -lha -R /var/log/journal
> /var/log/journal:
> insgesamt 20K
> drwxr-xr-x+ 3 root root 4,0K 31. Mär 01:46 .
> drwxr-xr-x. 3 root root 4,0K 31. Mär 01:46 ..
> drwxr-xr-x+ 2 root root 4,0K  2. Apr 02:07 74e76163c28448fba68b8667eb7b5d92
>
> /var/log/journal/74e76163c28448fba68b8667eb7b5d92:
> insgesamt 21M
> drwxr-xr-x+ 2 root root4,0K  2. Apr 02:07 .
> drwxr-xr-x+ 3 root root4,0K 31. Mär 01:46 ..
> -rw-r-+ 1 root systemd-journal 4,4M  2. Apr 00:12
> system@0004d953e9945a55-090715d148111d7e.journal~
> -rw-r-+ 1 root systemd-journal 4,2M  2. Apr 01:08
> system@0004d954b3c5c6e0-fe173546ebd601e1.journal~
> -rw-r-+ 1 root systemd-journal 4,2M  2. Apr 01:54
> system@0004d955565f8c45-d1fca3f8ff5062e1.journal~
> -rw-r-+ 1 root systemd-journal 4,2M  2. Apr 02:07
> system@0004d955873fdee1-dddc656c24d51746.journal~
> -rw-r-+ 1 root systemd-journal 4,1M  2. Apr 02:12 system.journal
>
>
> ___
> 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

[systemd-devel] [PATCH] Add some extra __attribute__ ((format)) s

2013-04-02 Thread Cristian Rodríguez 

diff --git a/src/core/manager.h b/src/core/manager.h
index 9d8d943..cd1ef23 100644
--- a/src/core/manager.h
+++ b/src/core/manager.h
@@ -301,6 +301,6 @@ void manager_undo_generators(Manager *m);
 void manager_recheck_journal(Manager *m);
 
 void manager_set_show_status(Manager *m, bool b);
-void manager_status_printf(Manager *m, bool ephemeral, const char *status, 
const char *format, ...);
+void manager_status_printf(Manager *m, bool ephemeral, const char *status, 
const char *format, ...) _printf_attr_(4,5);
 
 void watch_init(Watch *w);
diff --git a/src/journal/journald-server.h b/src/journal/journald-server.h
index 21edd6b..ffcf2f3 100644
--- a/src/journal/journald-server.h
+++ b/src/journal/journald-server.h
@@ -130,7 +130,7 @@ typedef struct Server {
 #define N_IOVEC_UDEV_FIELDS 32
 
 void server_dispatch_message(Server *s, struct iovec *iovec, unsigned n, 
unsigned m, struct ucred *ucred, struct timeval *tv, const char *label, size_t 
label_len, const char *unit_id, int priority);
-void server_driver_message(Server *s, sd_id128_t message_id, const char 
*format, ...);
+void server_driver_message(Server *s, sd_id128_t message_id, const char 
*format, ...) _printf_attr_(3,4);
 
 /* gperf lookup function */
 const struct ConfigPerfItem* journald_gperf_lookup(const char *key, unsigned 
length);
diff --git a/src/journal/microhttpd-util.h b/src/journal/microhttpd-util.h
index d4fefa7..36c7c10 100644
--- a/src/journal/microhttpd-util.h
+++ b/src/journal/microhttpd-util.h
@@ -23,4 +23,4 @@
 
 #include 
 
-void microhttpd_logger(void *arg, const char *fmt, va_list ap);
+void microhttpd_logger(void *arg, const char *fmt, va_list ap) 
__attribute__((format(printf, 2, 0)));
diff --git a/src/shared/log.h b/src/shared/log.h
index 9aafcb4..5fc8988 100644
--- a/src/shared/log.h
+++ b/src/shared/log.h
@@ -83,7 +83,7 @@ int log_metav(
 int line,
 const char *func,
 const char *format,
-va_list ap);
+va_list ap) _printf_attr_(5,0);
 
 int log_meta_object(
 int level,
@@ -102,14 +102,14 @@ int log_metav_object(
 const char *object_name,
 const char *object,
 const char *format,
-va_list ap);
+va_list ap) _printf_attr_(7,0);
 
 int log_struct_internal(
 int level,
 const char *file,
 int line,
 const char *func,
-const char *format, ...) _sentinel_;
+const char *format, ...) _printf_attr_(5,0) _sentinel_;
 
 int log_oom_internal(
 const char *file,
diff --git a/src/shared/util.h b/src/shared/util.h
index d1cdd90..78d8220 100644
--- a/src/shared/util.h
+++ b/src/shared/util.h
@@ -359,8 +359,8 @@ int pipe_eof(int fd);
 
 cpu_set_t* cpu_set_malloc(unsigned *ncpus);
 
-int status_vprintf(const char *status, bool ellipse, bool ephemeral, const 
char *format, va_list ap);
-int status_printf(const char *status, bool ellipse, bool ephemeral, const char 
*format, ...);
+int status_vprintf(const char *status, bool ellipse, bool ephemeral, const 
char *format, va_list ap) _printf_attr_(4,0);
+int status_printf(const char *status, bool ellipse, bool ephemeral, const char 
*format, ...) _printf_attr_(4,5);
 int status_welcome(void);
 
 int fd_columns(int fd);
diff --git a/src/systemd/sd-bus.h b/src/systemd/sd-bus.h
index 057931d..6afd79c 100644
--- a/src/systemd/sd-bus.h
+++ b/src/systemd/sd-bus.h
@@ -161,7 +161,7 @@ int sd_bus_get_owner_pid(sd_bus *bus, const char *name, 
pid_t *pid);
 #define SD_BUS_ERROR_INIT_CONST(name, message) {(name), (message), 0}
 
 void sd_bus_error_free(sd_bus_error *e);
-int sd_bus_error_set(sd_bus_error *e, const char *name, const char *format, 
...);
+int sd_bus_error_set(sd_bus_error *e, const char *name, const char *format, 
...)  __attribute__ ((format (printf, 3, 0)));
 void sd_bus_error_set_const(sd_bus_error *e, const char *name, const char 
*message);
 int sd_bus_error_copy(sd_bus_error *dest, const sd_bus_error *e);
 int sd_bus_error_is_set(const sd_bus_error *e);
diff --git a/src/systemd/sd-journal.h b/src/systemd/sd-journal.h
index 2e8d2d8..c624da1 100644
--- a/src/systemd/sd-journal.h
+++ b/src/systemd/sd-journal.h
@@ -38,15 +38,15 @@ extern "C" {
 
 /* Write to daemon */
 int sd_journal_print(int priority, const char *format, ...) __attribute__ 
((format (printf, 2, 3)));
-int sd_journal_printv(int priority, const char *format, va_list ap);
-int sd_journal_send(const char *format, ...) __attribute__((sentinel));
+int sd_journal_printv(int priority, const char *format, va_list ap) 
__attribute__ ((format (printf, 2, 0)));
+int sd_journal_send(const char *format, ...) __attribute__ ((format (printf, 
1, 0))) __attribute__((sentinel));
 int sd_journal_sendv(const struct iovec *iov, int n);
 int sd_journal_perror(const char *message);
 
 /* Used by the macros below. Don't call this directly. */
 int sd_journal_