Re: [PATCH 00/44] kdbus cleanups

[David Herrmann]

Hi

On Thu, Oct 8, 2015 at 1:31 PM, Sergei Zviagintsev <sergei@xxxxxxxx> wrote:
> Hi all,
>
> This is a set of various kdbus code cleanups. Patches are ordered by
> increasing complexity, starting with docs and comments fixes and
> one-liners.
>
> Patch 29 is the revised version of
> http://lkml.kernel.org/g/1435497454-10464-6-git-send-email-sergei@xxxxxxxx
>
> Feel free to ask to change layout of this, split/join, etc if necessary.

So I reviewed all of the patches, most of them look good. Some comments:
 - Please justify your changes in the commit-message. Always.
 - Please don't split patches based on modified functions. If you fix
typos, do them subsystem-wide. If you fix common errors like "don't
init 'name' to NULL before calling kdbus_pin_dst()", then please do
that for all functions. In other words, group your changes logically,
not based on location.
 - If you do cleanups, explain why you do them. I commented on some of
the changes, which IMO reduce readability.

Anyway, looks good.

Thanks a lot!
David

> Thanks, Sergei
>
> Sergei Zviagintsev (44):
>   Documentation/kdbus: Document new name registry flags
>   uapi: kdbus.h: Kernel-doc fixes
>   kdbus: Kernel-docs and comments trivial fixes
>   kdbus: Update kernel-doc for struct kdbus_pool
>   kdbus: Add comment on merging free pool slices
>   kdbus: Fix kernel-doc for struct kdbus_gaps
>   kdbus: Fix comment on translation of caps between namespaces
>   kdbus: Rename var in kdbus_meta_export_caps()
>   kdbus: Remove unused KDBUS_MSG_MAX_SIZE constant
>   kdbus: Use conditional operator
>   kdbus: Cosmetic fix of kdbus_name_is_valid()
>   kdbus: Use conventional list macros in __kdbus_pool_slice_release()
>   kdbus: Use list_next_entry() in kdbus_queue_entry_unlink()
>   kdbus: Simplify expression in kdbus_get_memfd()
>   kdbus: Simplify bitwise expression in kdbus_meta_get_mask()
>   kdbus: Drop redundant code from kdbus_name_acquire()
>   kdbus: Drop duplicated code from kdbus_pool_slice_alloc()
>   kdbus: Add var initialization to kdbus_conn_entry_insert()
>   kdbus: Drop useless initialization from kdbus_conn_reply()
>   kdbus: Drop useless initialization from kdbus_cmd_hello()
>   kdbus: Cleanup tests in kdbus_cmd_send()
>   kdbus: Cleanup error path in kdbus_staging_new_user()
>   kdbus: Cleanup kdbus_conn_call()
>   kdbus: Cleanup kdbus_conn_unicast()
>   kdbus: Cleanup kdbus_cmd_conn_info()
>   kdbus: Cleanup kdbus_pin_dst()
>   kdbus: Cleanup kdbus_conn_new()
>   kdbus: Cleanup kdbus_queue_entry_new()
>   kdbus: Improve tests on incrementing quota
>   kdbus: Cleanup kdbus_meta_proc_mask()
>   kdbus: Cleanup kdbus_conn_move_messages()
>   kdbus: Remove duplicated code from kdbus_conn_lock2()
>   kdbus: Improve kdbus_staging_reserve()
>   kdbus: Improve kdbus_conn_entry_sync_attach()
>   kdbus: Drop goto from kdbus_queue_entry_link()
>   kdbus: Improve kdbus_name_release()
>   kdbus: Fix error path in kdbus_meta_proc_collect_cgroup()
>   kdbus: Fix error path in kdbus_user_lookup()
>   kdbus: Cleanup kdbus_user_lookup()
>   kdbus: Cleanup kdbus_item_validate_name()
>   kdbus: Fix memfd install algorithm
>   kdbus: Check if fd is allocated before trying to free it
>   kdbus: Give up on failed fd allocation
>   kdbus: Cleanup kdbus_gaps_install()
>
>  Documentation/kdbus/kdbus.name.xml |  42 +++++++++-
>  include/uapi/linux/kdbus.h         |  43 +++++-----
>  ipc/kdbus/connection.c             | 157 +++++++++++++++----------------------
>  ipc/kdbus/connection.h             |  19 ++---
>  ipc/kdbus/domain.c                 |  38 +++++----
>  ipc/kdbus/fs.c                     |   2 +-
>  ipc/kdbus/item.c                   |  26 +++---
>  ipc/kdbus/limits.h                 |   3 -
>  ipc/kdbus/message.c                |  81 +++++++++----------
>  ipc/kdbus/message.h                |   9 ++-
>  ipc/kdbus/metadata.c               |  79 ++++++++++---------
>  ipc/kdbus/names.c                  |  32 ++++----
>  ipc/kdbus/node.c                   |   4 +-
>  ipc/kdbus/pool.c                   |  26 +++---
>  ipc/kdbus/queue.c                  |  51 ++++++------
>  ipc/kdbus/queue.h                  |   2 +-
>  16 files changed, 298 insertions(+), 316 deletions(-)
>
> --
> 1.8.3.1
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/