On Dienstag, 1. November 2016 20:07:24 CET Jean-Jacques Sarton wrote:
This patch shall allow to use the debugfs as previouly, including the check for valify of the interface used. For netns the check seem to be implemented within the function netlink_query_common(). For netlink we check if the originators ans transtable_global informations are accessible through netlink.
To: b.a.t.m.a.n@lists.open-mesh.org
Signed-off-by: Jean-Jacques Sarton jj.sarton@t-online.de
Please fix the subject/summary and write a proper commit message body [1]. Please remove the odd "To: " in the middle of the commit message body.
The subject of the mail should start with '[PATCH]' (or '[PATCH v2]' for the second version of the patch and so on). Then you should add "alfred: " so we easily spot in patchwork for which project the patch is. After that, you should add a "oneline summary" of what it is doing and not the problem like in your "Alfred not working within network name space".
About the commit message body:
* "This patch" is a bad start for a commit message * The first sentence doesn't make a lot of sense because your change should add the interface checks via netlink without using debugfs. * previouly -> previously * What is "valify"? * netlink_query_common doesn't check the validity of the interface but it can be used to check if this interface supports the netlink commands to retrieve the global translation table and the originators table * ans -> and
Maybe you can describe what is currently wrong and then how it gets fixed.
The mail subject ("oneline summary") without the '[PATCH...]' + the commit message body (till the first occurence of an '---' line) is later used to describe the change in git.
[...]
-int batadv_interface_check(const char *mesh_iface) +int batadv_interface_check_debugfs(const char *mesh_iface) { char full_path[MAX_PATH + 1]; FILE *f; @@ -166,6 +166,18 @@ int batadv_interface_check(const char *mesh_iface) return 0; }
Please mark this function static.
+int batadv_interface_check(const char *mesh_iface) +{
- int ret = 0;
- enable_net_admin_capability(1);
- ret = batadv_interface_check_netlink(mesh_iface);
- enable_net_admin_capability(0);
- if ( ret < 0 )
return batadv_interface_check_debugfs(mesh_iface);- return 0;
+}
Please use the same whitespace rules like in the rest of the code [2]. For example a newline after the variables block, no extra spaces inside the parenthesis and a newline after "return batadv_"...
It is also not required to initialize ret when you overwrite the initial value later without reading the initial value somewhere.
[...]
+int batadv_interface_check_netlink(const char *mesh_iface) +{
- struct get_tq_netlink_opts opts = {
.tq = 0,.found = false,.query_opts = {.err = 0,},- };
- int ret = 0;
- memset(&opts.mac, 0, ETH_ALEN);
- ret = netlink_query_common(mesh_iface, BATADV_CMD_GET_ORIGINATORS,
get_tq_netlink_cb, &opts.query_opts);- if (ret < 0)
return ret;- memset(&opts.mac, 0, ETH_ALEN);
- ret = netlink_query_common(mesh_iface, BATADV_CMD_GET_TRANSTABLE_GLOBAL,
get_tq_netlink_cb, &opts.query_opts);- if (ret < 0)
return ret;- return 0;
+}
See previous mails and the things mentioned above. It is also odd that you use sometimes 12 spaces (better use 1 tab + 4 spaces for changes to this project).
Kind regards, Sven
[1] https://www.kernel.org/doc/Documentation/SubmittingPatches [2] We try to use a non-strict version of https://www.kernel.org/doc/Documentation/CodingStyle