Re: [systemd-devel] [PATCH] core: fix event source annotations

[Lennart Poettering]

On Wed, 29.04.15 21:29, Mantas Mikulėnas (graw...@gmail.com) wrote:

> These looked like a mass-replace gone slightly wrong – two statements
> with no { }'s, and no error checking.
> ---
>  src/core/busname.c | 4 +++-
>  src/core/manager.c | 5 -
>  src/core/socket.c  | 3 ++-
>  3 files changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/src/core/busname.c b/src/core/busname.c
> index 48cc045..94db122 100644
> --- a/src/core/busname.c
> +++ b/src/core/busname.c
> @@ -291,13 +291,15 @@ static int busname_watch_fd(BusName *n) {
>  r = sd_event_source_set_enabled(n->starter_event_source, 
> SD_EVENT_ON);
>  else
>  r = sd_event_add_io(UNIT(n)->manager->event, 
> &n->starter_event_source, n->starter_fd, EPOLLIN, busname_dispatch_io, n);
> -(void) 
> sd_event_source_set_description(n->starter_event_source, "busname-starter");
> +
>  if (r < 0) {
>  log_unit_warning_errno(UNIT(n)->id, r, "Failed to watch 
> starter fd: %m");
>  busname_unwatch_fd(n);
>  return r;
>  }
>  
> +(void) sd_event_source_set_description(n->starter_event_source, 
> "busname-starter");
> +

Hmm, it sounds a bit unnecessary to set the same description over and
over again. We should really only set it when we allocate a new event
source, and not every single time we toggle is enablement state.

> --- a/src/core/socket.c
> +++ b/src/core/socket.c
> @@ -1272,11 +1272,12 @@ static int socket_watch_fds(Socket *s) {
>  else
>  r = sd_event_add_io(UNIT(s)->manager->event, 
> &p->event_source, p->fd, EPOLLIN, socket_dispatch_io, p);
>  
> -(void) 
> sd_event_source_set_description(p->event_source, "socket-port-io");
>  if (r < 0) {
>  log_unit_warning_errno(UNIT(s)->id, r, "Failed to 
> watch listening fds: %m");
>  goto fail;
>  }
> +
> +(void) sd_event_source_set_description(p->event_source, 
> "socket-port-io");

Same here...

I fixed that now.

Lennart

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

Re: [systemd-devel] [PATCH] core: fix event source annotations

[Tom Gundersen]

Applied. Thanks!

Tom

On Wed, Apr 29, 2015 at 8:29 PM, Mantas Mikulėnas  wrote:
> These looked like a mass-replace gone slightly wrong – two statements
> with no { }'s, and no error checking.
> ---
>  src/core/busname.c | 4 +++-
>  src/core/manager.c | 5 -
>  src/core/socket.c  | 3 ++-
>  3 files changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/src/core/busname.c b/src/core/busname.c
> index 48cc045..94db122 100644
> --- a/src/core/busname.c
> +++ b/src/core/busname.c
> @@ -291,13 +291,15 @@ static int busname_watch_fd(BusName *n) {
>  r = sd_event_source_set_enabled(n->starter_event_source, 
> SD_EVENT_ON);
>  else
>  r = sd_event_add_io(UNIT(n)->manager->event, 
> &n->starter_event_source, n->starter_fd, EPOLLIN, busname_dispatch_io, n);
> -(void) 
> sd_event_source_set_description(n->starter_event_source, "busname-starter");
> +
>  if (r < 0) {
>  log_unit_warning_errno(UNIT(n)->id, r, "Failed to watch 
> starter fd: %m");
>  busname_unwatch_fd(n);
>  return r;
>  }
>
> +(void) sd_event_source_set_description(n->starter_event_source, 
> "busname-starter");
> +
>  return 0;
>  }
>
> diff --git a/src/core/manager.c b/src/core/manager.c
> index 0c94e9e..cf7337e 100644
> --- a/src/core/manager.c
> +++ b/src/core/manager.c
> @@ -90,6 +90,7 @@ static void manager_undo_generators(Manager *m);
>
>  static void manager_watch_jobs_in_progress(Manager *m) {
>  usec_t next;
> +int r;
>
>  assert(m);
>
> @@ -97,12 +98,14 @@ static void manager_watch_jobs_in_progress(Manager *m) {
>  return;
>
>  next = now(CLOCK_MONOTONIC) + JOBS_IN_PROGRESS_WAIT_USEC;
> -(void) sd_event_add_time(
> +r = sd_event_add_time(
>  m->event,
>  &m->jobs_in_progress_event_source,
>  CLOCK_MONOTONIC,
>  next, 0,
>  manager_dispatch_jobs_in_progress, m);
> +if (r < 0)
> +return;
>
>  (void) 
> sd_event_source_set_description(m->jobs_in_progress_event_source, 
> "manager-jobs-in-progress");
>  }
> diff --git a/src/core/socket.c b/src/core/socket.c
> index 702742f..67beda4 100644
> --- a/src/core/socket.c
> +++ b/src/core/socket.c
> @@ -1272,11 +1272,12 @@ static int socket_watch_fds(Socket *s) {
>  else
>  r = sd_event_add_io(UNIT(s)->manager->event, 
> &p->event_source, p->fd, EPOLLIN, socket_dispatch_io, p);
>
> -(void) 
> sd_event_source_set_description(p->event_source, "socket-port-io");
>  if (r < 0) {
>  log_unit_warning_errno(UNIT(s)->id, r, "Failed to 
> watch listening fds: %m");
>  goto fail;
>  }
> +
> +(void) sd_event_source_set_description(p->event_source, 
> "socket-port-io");
>  }
>
>  return 0;
> --
> 2.3.7
>
> ___
> 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] core: fix event source annotations

[Mantas Mikulėnas]

These looked like a mass-replace gone slightly wrong – two statements
with no { }'s, and no error checking.
---
 src/core/busname.c | 4 +++-
 src/core/manager.c | 5 -
 src/core/socket.c  | 3 ++-
 3 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/src/core/busname.c b/src/core/busname.c
index 48cc045..94db122 100644
--- a/src/core/busname.c
+++ b/src/core/busname.c
@@ -291,13 +291,15 @@ static int busname_watch_fd(BusName *n) {
 r = sd_event_source_set_enabled(n->starter_event_source, 
SD_EVENT_ON);
 else
 r = sd_event_add_io(UNIT(n)->manager->event, 
&n->starter_event_source, n->starter_fd, EPOLLIN, busname_dispatch_io, n);
-(void) 
sd_event_source_set_description(n->starter_event_source, "busname-starter");
+
 if (r < 0) {
 log_unit_warning_errno(UNIT(n)->id, r, "Failed to watch 
starter fd: %m");
 busname_unwatch_fd(n);
 return r;
 }
 
+(void) sd_event_source_set_description(n->starter_event_source, 
"busname-starter");
+
 return 0;
 }
 
diff --git a/src/core/manager.c b/src/core/manager.c
index 0c94e9e..cf7337e 100644
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -90,6 +90,7 @@ static void manager_undo_generators(Manager *m);
 
 static void manager_watch_jobs_in_progress(Manager *m) {
 usec_t next;
+int r;
 
 assert(m);
 
@@ -97,12 +98,14 @@ static void manager_watch_jobs_in_progress(Manager *m) {
 return;
 
 next = now(CLOCK_MONOTONIC) + JOBS_IN_PROGRESS_WAIT_USEC;
-(void) sd_event_add_time(
+r = sd_event_add_time(
 m->event,
 &m->jobs_in_progress_event_source,
 CLOCK_MONOTONIC,
 next, 0,
 manager_dispatch_jobs_in_progress, m);
+if (r < 0)
+return;
 
 (void) 
sd_event_source_set_description(m->jobs_in_progress_event_source, 
"manager-jobs-in-progress");
 }
diff --git a/src/core/socket.c b/src/core/socket.c
index 702742f..67beda4 100644
--- a/src/core/socket.c
+++ b/src/core/socket.c
@@ -1272,11 +1272,12 @@ static int socket_watch_fds(Socket *s) {
 else
 r = sd_event_add_io(UNIT(s)->manager->event, 
&p->event_source, p->fd, EPOLLIN, socket_dispatch_io, p);
 
-(void) 
sd_event_source_set_description(p->event_source, "socket-port-io");
 if (r < 0) {
 log_unit_warning_errno(UNIT(s)->id, r, "Failed to 
watch listening fds: %m");
 goto fail;
 }
+
+(void) sd_event_source_set_description(p->event_source, 
"socket-port-io");
 }
 
 return 0;
-- 
2.3.7

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