[systemd-devel] [PATCH] libsystemd-bus: Clean up code

[Zbigniew Jędrzejewski-Szmek]

On Mon, Dec 09, 2013 at 09:22:43PM +0100, Thomas H.P. Andersen wrote:
> If I understood correctly then the assert_return was meant to be used
> only on the public library functions. I can't seem to find the
> reference to it so maybe I am wrong though.
There's no strong reason to limit usage to public functions. It's just
a simple macro really.

> On Mon, Dec 9, 2013 at 2:09 PM, Lukasz Skalski
> <l.skalski at partner.samsung.com> wrote:
> > ---
> >  src/libsystemd-bus/bus-dump.c      |  2 +-
> >  src/libsystemd-bus/bus-error.c     |  3 +--
> >  src/libsystemd-bus/bus-kernel.c    | 12 +++---------
> >  src/libsystemd-bus/bus-message.c   | 12 +++---------
> >  src/libsystemd-bus/bus-signature.c | 13 ++++---------
> >  5 files changed, 12 insertions(+), 30 deletions(-)
> >
> > diff --git a/src/libsystemd-bus/bus-dump.c b/src/libsystemd-bus/bus-dump.c
> > index ddad418..a999683 100644
> > --- a/src/libsystemd-bus/bus-dump.c
> > +++ b/src/libsystemd-bus/bus-dump.c
> > @@ -56,7 +56,7 @@ int bus_message_dump(sd_bus_message *m, FILE *f, bool with_header) {
> >
> >          if (with_header) {
> >                  fprintf(f,
> > -                        "%s%s%sType=%s%s%s  Endian=%c  Flags=%u  Version=%u",
> > +                        "%s%s%sType=%s%s%s  Endian=%c  Flags=%u  Version=%u ",
I don't get this part, since a space is added in the messages right below.

> >                          m->header->type == SD_BUS_MESSAGE_METHOD_ERROR ? ansi_highlight_red() :
> >                          m->header->type == SD_BUS_MESSAGE_METHOD_RETURN ? ansi_highlight_green() :
> >                          m->header->type != SD_BUS_MESSAGE_SIGNAL ? ansi_highlight() : "", draw_special_char(DRAW_TRIANGULAR_BULLET), ansi_highlight_off(),
> > diff --git a/src/libsystemd-bus/bus-error.c b/src/libsystemd-bus/bus-error.c
> > index 25eaf0e..4f18629 100644
> > --- a/src/libsystemd-bus/bus-error.c
> > +++ b/src/libsystemd-bus/bus-error.c
> > @@ -39,8 +39,7 @@ static int bus_error_name_to_errno(const char *name) {
> >          const char *p;
> >          int r;
> >
> > -        if (!name)
> > -                return EINVAL;
> > +        assert_return(name, EINVAL);
> >
> >          p = startswith(name, "System.Error.");
> >          if (p) {
> > diff --git a/src/libsystemd-bus/bus-kernel.c b/src/libsystemd-bus/bus-kernel.c
> > index 495d7e5..d5574ce 100644
> > --- a/src/libsystemd-bus/bus-kernel.c
> > +++ b/src/libsystemd-bus/bus-kernel.c
> > @@ -321,9 +321,7 @@ int bus_kernel_take_fd(sd_bus *b) {
> >          int r;
> >
> >          assert(b);
> > -
> > -        if (b->is_server)
> > -                return -EINVAL;
> > +        assert_return(!b->is_server, -EINVAL);
> >
> >          b->use_memfd = 1;
> >
> > @@ -374,9 +372,7 @@ int bus_kernel_connect(sd_bus *b) {
> >          assert(b->input_fd < 0);
> >          assert(b->output_fd < 0);
> >          assert(b->kernel);
> > -
> > -        if (b->is_server)
> > -                return -EINVAL;
> > +        assert_return(!b->is_server, -EINVAL);
> >
> >          b->input_fd = open(b->kernel, O_RDWR|O_NOCTTY|O_CLOEXEC);
> >          if (b->input_fd < 0)
> > @@ -904,9 +900,7 @@ int bus_kernel_pop_memfd(sd_bus *bus, void **address, size_t *size) {
> >
> >          assert(address);
> >          assert(size);
> > -
> > -        if (!bus || !bus->is_kernel)
> > -                return -ENOTSUP;
> > +        assert_return(bus || bus->is_kernel, -ENOTSUP);
> You should && them here.
Good catch. Fixed up and applied.

Zbyszek