diff mbox

remove two compiler warnings for unused variables

Message ID 1460069165-5638-1-git-send-email-nhofmeyr@sysmocom.de
State Not Applicable
Headers show

Commit Message

Neels Hofmeyr April 7, 2016, 10:46 p.m. UTC
---
 src/sua.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Harald Welte April 11, 2016, 10:23 p.m. UTC | #1
Hi Max,

I disagree with this patch.  We don't want to make all warnings quiet at
all cost.  If that was the goal, we could simply disable compiler
warnings and go ahead.

The point is that the SUA code is incomplete, and I prefer to have a
warning about it until it gets properly fixed.

This:

On Fri, Apr 08, 2016 at 12:46:05AM +0200, Neels Hofmeyr wrote:
> -	cause = xua_msg_get_u32(xua, SUA_IEI_CAUSE);
> +	/* TODO evaluate cause:
> +	 * cause = xua_msg_get_u32(xua, SUA_IEI_CAUSE); */

will just mean that we all forget about having to fix it.
Neels Hofmeyr April 12, 2016, 11:17 a.m. UTC | #2
On Tue, Apr 12, 2016 at 12:23:52AM +0200, Harald Welte wrote:
> Hi Max,

It was me, not Max :)

> will just mean that we all forget about having to fix it.

Maybe instead of numerous unused variables, we should have a single
#warning then. Every warning hinders my dev cycle. For pretty much every
compilation, I have to parse/browse the list of warnings, *all* the time.

Even a #warning would stare me in the face for every build. There are a
couple of those already. So I'd even prefer a TODO file in / , admitted
that these kind of files tend to go outdated.

Also, having warnings around makes it harder to spot new warnings I've
introduced. I see some warnings scroll by, then assume they're just the
old ones, or have to scroll around to check every time.

I'd prefer a way that I don't have to check for and read the same old
warnings dozens of times every day, while not having the time to
investigate/fix anyway... I unnecessarily lose time on those.

A workaround for me would be to eradicate the warnings on our iu branches
but not commit them to master.

~Neels
Harald Welte April 12, 2016, 8:49 p.m. UTC | #3
Hi Neels,

On Tue, Apr 12, 2016 at 01:17:34PM +0200, Neels Hofmeyr wrote:
> On Tue, Apr 12, 2016 at 12:23:52AM +0200, Harald Welte wrote:
> > Hi Max,
> 
> It was me, not Max :)

Sorry.

> > will just mean that we all forget about having to fix it.
> 
> Maybe instead of numerous unused variables, we should have a single
> #warning then. Every warning hinders my dev cycle. For pretty much every
> compilation, I have to parse/browse the list of warnings, *all* the time.

Please let me know after how many compilatiosn the trade-off woudl trip
towards actually addressing those warnings in fixing the related code.

> Even a #warning would stare me in the face for every build. There are a
> couple of those already. 

Well, if everyone every so often would address one of them, it wouldn't
be so bad :)
diff mbox

Patch

diff --git a/src/sua.c b/src/sua.c
index 6e187d8..df60c13 100644
--- a/src/sua.c
+++ b/src/sua.c
@@ -793,7 +793,6 @@  static int sua_rx_core(struct osmo_sua_link *link, struct xua_msg *xua)
 	struct xua_msg_part *data_ie = xua_msg_find_tag(xua, SUA_IEI_DATA);
 	struct msgb *upmsg;
 	struct sua_connection *conn;
-	uint8_t *cur;
 
 	/* fill conn */
 	conn = conn_create(link);
@@ -926,7 +925,8 @@  static int sua_rx_coref(struct osmo_sua_link *link, struct xua_msg *xua)
 	memcpy(&param->calling_addr, &conn->calling_addr,
 		sizeof(param->calling_addr));
 	//param->in_sequence_control;
-	cause = xua_msg_get_u32(xua, SUA_IEI_CAUSE);
+	/* TODO evaluate cause:
+	 * cause = xua_msg_get_u32(xua, SUA_IEI_CAUSE); */
 	/* optional: src addr */
 	/* optional: dest addr */
 	param->importance = xua_msg_get_u32(xua, SUA_IEI_IMPORTANCE);