Bug 2548

Summary: Dead assignment on lte-enb-mac.cc
Product: ns-3 Reporter: natale.patriciello
Component: lteAssignee: Biljana Bojović <bbojovic>
Status: RESOLVED FIXED    
Severity: minor CC: mmiozzo, ns-bugs, tomh
Priority: P5    
Version: unspecified   
Hardware: PC   
OS: Linux   

Description natale.patriciello 2016-11-08 03:51:07 UTC
The snippet is the following:

457	  // --- DOWNLINK ---
458	  // Send Dl-CQI info to the scheduler
459	  if (m_dlCqiReceived.size () > 0)
460	    {
461	      FfMacSchedSapProvider::SchedDlCqiInfoReqParameters dlcqiInfoReq;
462	      dlcqiInfoReq.m_sfnSf = ((0x3FF & frameNo) << 4) | (0xF & subframeNo);
463	 
464	      int cqiNum = m_dlCqiReceived.size ();
465	      if (cqiNum > MAX_CQI_LIST)
466	        {
467	          cqiNum = MAX_CQI_LIST; // Value stored to 'cqiNum' is never read
468	        }
469	      dlcqiInfoReq.m_cqiList.insert (dlcqiInfoReq.m_cqiList.begin (), m_dlCqiReceived.begin (), m_dlCqiReceived.end ());
470	      m_dlCqiReceived.erase (m_dlCqiReceived.begin (), m_dlCqiReceived.end ());
471	      m_schedSapProvider->SchedDlCqiInfoReq (dlcqiInfoReq);
472	    }

At line 467, the value stored to cqiNum is never read (dead assignment). I suppose that it was coded to impose an "hard limit" on the number of object copied at line 469. However, that limit is not honoured. The problem in using m_dlCqiReceived.end () at line 469, is if this value is greater than MAX_CQI_LIST.

I don't know what is the right behaviour here; if is the existing one, from line 465 to line 468 the code can be removed, otherwise a for-loop to MAX_CQI_LIST should be used to copy elements.
Comment 1 Marco Miozzo 2016-11-08 06:56:18 UTC
MAX_CQI_LIST it's a magic number coming from the SCF APIs. This code does not affect  the behavior of the simulator, but definitely it is a dead assignment. This kind of requirements usually come from HW requirements. In this case, it limits the number of UEs that can report CQIs per subframe to the scheduler. According to SCF APIs, MAX_CQI_LIST is set to 30, which is a quite high number of UEs, especially considering that at most 25 UEs can be allocated with a 20 MHz channel and with allocation type 0 (i.e., 1 RBG per UE). I'm not expert on D2D LTE communication, but in that case might be larger, I guess.
Considering standard LTE R-8/10, we can put an assert to better mimic the desired behavior, e.g.:
NS_ASSERT (m_dlCqiReceived.size () <= MAX_CQI_LIST);
Comment 2 Tom Henderson 2017-04-17 20:26:33 UTC
was fixed a while back in changeset 12736:88d603958635