Skip to content
This repository has been archived by the owner on Dec 27, 2019. It is now read-only.

tools: Rename IPC functions to reduce misleading #6

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

liuqun
Copy link

@liuqun liuqun commented Mar 1, 2019

The function name like ipc_get_device() is a little bit misleading.
To make the code more readable for new developers, here are some suggestions:

  • Rename ipc_get_device(&dev) to ipc_fetch_conf(&conf) because it only fetches the runtime configuations from kernel.
  • Funtion prototype ipc_set_device(struct wgdevice *dev) is changed to ipc_set_device(const struct wgdevice *newconf).

Change the following IPC function names and parameters which used to be confusing:

  • ipc_set_device(dev) => ipc_set_device(newconf)
  • ipc_get_device(dev, interface) => ipc_fetch_conf(conf, interface)
  • kernel_set_device(dev) => kernel_set_device(newconf)
  • kernel_get_device(device, interface) => kernel_fetch_conf(conf, interface)
  • userspace_set_device(dev) => userspace_set_device(newconf)
  • userspace_get_device(out, interface) => userspace_fetch_conf(out, interface)

Renamed local variable/parameter symbols:

  • In showconf_main(), show_main():
    device => conf
  • In ipc_fetch_conf(), kernel_fetch_conf(), userspace_fetch_conf():
    dev => conf
  • In ipc_set_device(), kernel_set_device(), userspace_set_device():
    dev => newconf

Added function:

  • free_conf(struct wgdevice *conf)

Changed function prototypes:

  • int ipc_set_device(struct wgdevice *)
    => int ipc_set_device(const struct wgdevice *newconf)
  • int kernel_set_device(struct wgdevice *)
    => int kernel_set_device(const struct wgdevice *newconf)
  • int userspace_set_device(struct wgdevice *)
    => int userspace_set_device(const struct wgdevice *newconf)

Note: This patch comes with no extra curly braces introduced.

-- Liu Qun

@liuqun
Copy link
Author

liuqun commented Mar 1, 2019

@thinrope

In the Linux kernel source tree, there is one code formater script called Lindent.
But I am not quite sure how it works. I can see it calls indent interally with many code formating sytle options. Hope that I could figure out how to use it some day.

One blog: Lindent – amazing script for lazy C coder

image

Another discussion on Lindent suggest us to use indent --linux-style instead of the script Lindent:

... recent versions of indent (as of November 2007) have a "--linux-style" option which serves the same purpose as Lindent. The result is almost the same in my case, but --linux-style translates to a much longer list of options. Also some options seem better than what Lindent uses (e.g. -il1 instead of -il0.)

In the light of this I would tend to agree with Joe that Lindent could go away and references to it be replaced with "indent --linux-style".
-- said Jean Delvare at https://lore.kernel.org/patchwork/patch/577917/

By the way, perhaps I should just follow other kernel developers' current coding style. As an old phrase says "When in Rome, do as Romans do"...

src/tools/showconf.c Outdated Show resolved Hide resolved
The function name like ipc_get_device() is a little bit misleading.
To make the code more readable for new developers, here are some suggestions:
* Rename ipc_get_device(&dev) to ipc_fetch_conf(&conf) because it only fetches the runtime configuations from kernel.
* Funtion prototype ipc_set_device(struct wgdevice *dev) is changed to ipc_set_device(const struct wgdevice *newconf).

Change the following IPC function names and parameters which used to be confusing:
* ipc_set_device(dev)                  => ipc_set_device(newconf)
* ipc_get_device(dev, interface)       => ipc_fetch_conf(conf, interface)
* kernel_set_device(dev)               => kernel_set_device(newconf)
* kernel_get_device(device, interface) => kernel_fetch_conf(conf, interface)
* userspace_set_device(dev)            => userspace_set_device(newconf)
* userspace_get_device(out, interface) => userspace_fetch_conf(out, interface)

Renamed local variable/parameter symbols:
* In showconf_main(), show_main():
    device => conf
* In ipc_fetch_conf(), kernel_fetch_conf(), userspace_fetch_conf():
    dev => conf
* In ipc_set_device(), kernel_set_device(), userspace_set_device():
    dev => newconf

Added function:
* free_conf(struct wgdevice *conf)

Changed function prototypes:
* int ipc_set_device(struct wgdevice *)
=> int ipc_set_device(const struct wgdevice *newconf)
* int kernel_set_device(struct wgdevice *)
=> int kernel_set_device(const struct wgdevice *newconf)
* int userspace_set_device(struct wgdevice *)
=> int userspace_set_device(const struct wgdevice *newconf)

Note: This patch comes with no extra curly braces introduced.

Signed-off-by: Liu Qun <[email protected]>
@liuqun liuqun force-pushed the tools_refactor_ipc branch from a163612 to c35d63a Compare March 1, 2019 08:13
@zx2c4-bot zx2c4-bot force-pushed the master branch 2 times, most recently from 26f936d to 20c713b Compare April 23, 2019 11:13
@zx2c4-bot zx2c4-bot force-pushed the master branch 7 times, most recently from 2a831c3 to 478b80a Compare May 31, 2019 16:31
@zx2c4-bot zx2c4-bot force-pushed the master branch 14 times, most recently from 814d7b7 to a2512b1 Compare June 19, 2019 18:17
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants