Re: [PATCH] init and extra checking fixups
From: Johan Hedberg <hidden>
Date: 2009-09-27 08:41:51
Hi Steve, On Sat, Sep 26, 2009, Steve Grubb wrote:
In audio/avdtp.c, the avdtp_setconf_cmd function has an exit jump for failed. There are several calls to the goto that do not set err to something, meaning that the stack contents are what's used. I set err =0 in the beginning so that we no longer use the stack contents, but it seems like there should be some general error code that says we failed for an unspecified reason.
quoted hunk ↗ jump to hunk
diff -urp bluez-4.54.orig/audio/avdtp.c bluez-4.54/audio/avdtp.c--- bluez-4.54.orig/audio/avdtp.c 2009-09-26 08:43:56.000000000 -0400 +++ bluez-4.54/audio/avdtp.c 2009-09-26 11:17:07.000000000 -0400@@ -1248,7 +1248,7 @@ static gboolean avdtp_setconf_cmd(struct struct conf_rej rej; struct avdtp_local_sep *sep; struct avdtp_stream *stream; - uint8_t err, category = 0x00; + uint8_t err = 0, category = 0x00; struct audio_device *dev; bdaddr_t src, dst; GSList *l;
The right error for the two places is AVDTP_BAD_STATE. Setting that error before jumping to failed also means that we avoid the initialization upon declaration for the err variable. David and Luiz commented already on a few other issues which would be good to fix too. Furthermore, don't be afraid to split your patches into smaller logical chunks. That way some of them could have been already applied and pushed upstream. And please leave out the Signed-off-by line since we don't use it in bluez. Johan