inet-framework / inet

INET Framework for the OMNeT++ discrete event simulator
https://inet.omnetpp.org
Other
440 stars 486 forks source link

dupShare can return a immutable object #534

Closed aarizaq closed 4 years ago

aarizaq commented 4 years ago

Chunk dupShared can return a immutable object, the copy operator copy the flags and doesn't modify the mutable flags. Possible solution: https://github.com/aarizaq/inetmanet-4.x/commit/dfb6647ac801c3acce8c3225464edea7b2cd689e

levy commented 4 years ago

Actually the copy constructor of Chunk is implemented as (in master):

Chunk::Chunk(const Chunk& other) : id(nextId++), flags(other.flags & ~CF_IMMUTABLE), tags(other.tags) { }

So could you please be more specific on what problem you found? I'm not saying there isn't one, but maybe it's different.

aarizaq commented 4 years ago

In my case, removeAtFront returned in a case a inmutable header, the flag inmutable was active

levy commented 4 years ago

Hmm, that should not be the case. Could you narrow the code down to a simple example?

aarizaq commented 4 years ago

imagen

auto pdu = pkt->removeAtFront<LteMacPdu>();
pdu_ = pkt;
pduId_ = pdu->getId();
// as unique MacPDUId the OMNET id is used (need separate member since it must not be changed by dup())
pdu->setMacPduId(pdu->getId());
aarizaq commented 4 years ago

the error is throw in setMacPduId() I discovered that the flag was not reset in dupShared

levy commented 4 years ago

I suspect that the copy ctor of LteMacPdu doesn't call down to the Chunk copy ctor. All packet parts must subclass inet::Chunk and properly call the constructors. At least, this is my theory.

aarizaq commented 4 years ago

That may be the problem, I was adapting the SimuLte and I it quite sure that I have forget to include the copy of FieldChunk

aarizaq commented 4 years ago

No, it call the operator copy ::inet::FieldsChunk::operator=(other);

levy commented 4 years ago

Calling the operator= from the copy ctor is not right. The copy ctor supposed to call the copy ctors all the way down.

BTW, there's an ongoing effort to port SimuLTE to INET 4.x and it's already working to a large extent, I think. See here https://github.com/inet-framework/simulte/pull/45

aarizaq commented 4 years ago

The copy was created by the msg compiler, in any case I need to see the port

levy commented 4 years ago

As far as I see, the MSG compiler calls the copy() method from both the copy constructor and the operator=. The copy() method doesn't call the base class copy() method, and that is right. Also, generated copy constructors always call the base class copy constructors. At least that's the case with the latest omnet, but I'm not sure about the older versions though.

aarizaq commented 4 years ago

I am using omnet 5.6.1 in windows, now I can run the port that I have of LTE with inet 4.2 if I include the modification in dupShared. In any case, I will research a bit more trying to understand why the constructor doesn't reset the flag, it is possible that I have chosen the easier solution in this case instead of the best, now it is more curiosity that other thing. I like programming, and the confinement has forced us to search some entertainments to pass the time, and I do not like too much the TV.

levy commented 4 years ago

Well, then we are very much alike, because I also like programming and I also don't like watching TV. :)

aarizaq commented 4 years ago

The copy constructor was erroneous, and it also copied the flag. with a bit of patient, it has taken a bit more time because the debug continuously crashed in windows, util I have had the opportunity of to use a linux machine I haven’t had the opportunity to find the problem

aarizaq commented 4 years ago

The version of LTE that I have try to adapt is a bit more aggresive than net-framework/simulte#45 I have replaced all the previous packet formats and control info types for Chunks and Tags