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 <[email protected]> > >> > >> Initially developed by Linus during a 6 months trainee study > >> period in Ascom (Switzerland) AG. > >> > >> Signed-off-by: Linus Luessing <[email protected]> > >> Signed-off-by: Marek Lindner <[email protected]> > >> Signed-off-by: Antonio Quartulli <[email protected]> > >> --- > >> 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.
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;
?
--
Lew Pitcher
"In Skills, We Trust"
PGP public key available upon request
signature.asc
Description: This is a digitally signed message part.
