On Fri, Apr 15, 2016 at 11:01:04PM +0200, Neels Hofmeyr wrote:
On Thu, Apr 14, 2016 at 09:37:52AM -0400, Holger
Freyther wrote:
On 14 Apr 2016, at 09:21, Neels Hofmeyr
<nhofmeyr(a)sysmocom.de> wrote:
Fixing a few NULL dereference warnings found by the Iu coverity check.
They are not related apart from that.
I just looked at the gprs_gmm patch and stopped reading. Your commit message should at
least have some of the context of coverity.
So when is llme NULL? Is it allowed to be NULL? Does it make sense? We don't want to
blindly do these things but understand the code around and see if the tool is right or
wrong. And if we disagree maybe change the flow of code or add an assert.
Spot on, I don't actually understand the llme one. All I know is that the
calling function gsm0408_rcv_gmm() has a condition "if (llme..." up at the
top.
Let's drop that one unless someone else has the time to look at it.
I just noticed that on the Iu branch and for Iu connections, llme is explicitly
passed as NULL, so my patch would break things for Iu. Good thing you spotted
that it's fishy.
The patch should probably be
[[[
diff --git a/openbsc/src/gprs/gprs_gmm.c b/openbsc/src/gprs/gprs_gmm.c
index f510e64..f8d75d5 100644
--- a/openbsc/src/gprs/gprs_gmm.c
+++ b/openbsc/src/gprs/gprs_gmm.c
@@ -1185,7 +1185,7 @@ rejected:
rc = gsm48_tx_gmm_att_rej_oldmsg(msg, reject_cause);
if (ctx)
mm_ctx_cleanup_free(ctx, "GPRS ATTACH REJ");
- else
+ else if (llme)
/* TLLI unassignment */
gprs_llgmm_assign(llme, llme->tlli, 0xffffffff, GPRS_ALGO_GEA0, NULL);
]]]
and I would appreciate if someone else could verify that.
~Neels