On 11/02/14 17:11, Lew Pitcher wrote:
On Tuesday 11 February 2014 11:02:11 Antonio Quartulli wrote:
On 11/02/14 16:32, Andrew Lunn wrote:
On Tue, Feb 11, 2014 at 01:48:04PM +0100, Antonio Quartulli wrote:
From: Linus Luessing linus.luessing@web.de
Initially developed by Linus during a 6 months trainee study period in Ascom (Switzerland) AG.
Signed-off-by: Linus Luessing linus.luessing@web.de Signed-off-by: Marek Lindner lindner_marek@yahoo.de Signed-off-by: Antonio Quartulli antonio@open-mesh.com
bat_v.c | 18 +++++- bat_v_elp.c | 204
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
bat_v_elp.h | 6 ++ main.h | 2 + types.h | 53 ++++++++++++++++ 5 files changed, 281 insertions(+), 2 deletions(-)
diff --git a/bat_v.c b/bat_v.c index 7247d7f..bed5e00 100644 --- a/bat_v.c +++ b/bat_v.c @@ -61,5 +61,21 @@ static struct batadv_algo_ops batadv_batman_v
__read_mostly = {
int __init batadv_v_init(void) {
- return batadv_algo_register(&batadv_batman_v);
- int ret;
- /* batman v echo location protocol packet */
- ret = batadv_recv_handler_register(BATADV_ELP,
batadv_v_elp_packet_recv);- if (ret < 0)
goto elp_unregister;- ret = batadv_algo_register(&batadv_batman_v);
- return ret;
+elp_unregister:
- if (ret < 0)
batadv_recv_handler_unregister(BATADV_ELP);No need to check ret here. If we are here, it has to be < 0.
It also seems odd to me you are unregistering the handler when the registration of the handler fails!
I suspect the first if (ret < 0) should be followed by a plain return ret; and there should be a second test for the return value of batadv_algo_register() which should goto the label and unregister the handler.
I totally agree with what you said. We should jump to elp_unregister if batadv_algo_register() fails.
Sorry to break in here, but the (ex) professional programmer in me just /has/ to comment.
Everybody is welcome! :)
Why not just int __init batadv_v_init(void) {
- return batadv_algo_register(&batadv_batman_v);
- int ret;
- /* batman v echo location protocol packet */
- ret = batadv_recv_handler_register(BATADV_ELP,
batadv_v_elp_packet_recv);- if (ret >= 0)
ret = batadv_algo_register(&batadv_batman_v);- else
batadv_recv_handler_unregister(BATADV_ELP);- return ret;
?
Actually I already fixed it like this:
int __init batadv_v_init(void) { - return batadv_algo_register(&batadv_batman_v); + int ret; + + /* batman v echo location protocol packet */ + ret = batadv_recv_handler_register(BATADV_ELP, + batadv_v_elp_packet_recv); + if (ret < 0) + return ret; + + ret = batadv_algo_register(&batadv_batman_v); + if (ret < 0) + batadv_recv_handler_unregister(BATADV_ELP); + + return ret;
I think it is easier to read, no? Anyway, this chunk is changed later b patch 7/23 because we add the OGM2 registration.
Cheers,