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

[Lukasz Skalski]

On 12/10/2013 05:24 AM, Zbigniew Jędrzejewski-Szmek wrote:
> 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.
>

There should be two blank spaces (between Version and Serial fields, 
like between Type, Endian and Flags), so this part is also ok.

>>>                           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.

Thanks.

>
> Zbyszek
>

-- 
Lukasz Skalski
Samsung R&D Institute Poland
Samsung Electronics
l.skalski at partner.samsung.com