-1

I'm using libtins to capture packets and moodycamel Concurrent Queue to queue captured packets.

I note that the dequeue operation fails because PDU is an abstract type.

Hence it fails to compile,

error: cannot declare variable ‘pdu’ to be of abstract type ‘Tins::PDU’

I am not sure what to do at this stage to fix this error.

I've attempted the pointer route but it didn't compile too

    Tins::PDU *pdu;

    if(PacketQueue.try_dequeue(pdu) == false) {
        continue;
    }

The error,

 error: cannot convert ‘std::remove_reference<Tins::PDU&>::type {aka Tins::PDU}’ to ‘Tins::PDU*’ in assignment
       element = std::move(el); // NOLINT
       ~~~~~~~~^~~~~~~~~~~~~~~

Code

moodycamel::ConcurrentQueue<PDU> PacketQueue;

void worker()
{
    while(true) {
        Tins::PDU pdu;

        if(PacketQueue.try_dequeue(pdu) == false) {
            continue;
        }


        // Do Work
    }
}

bool callback(PDU &pdu)
{
  PacketQueue.enqueue(pdu);

  return true;
}
jeffbRTC
  • 1,941
  • 10
  • 29
  • 2
    If you have polymorphic classes then you need to use *pointers* to the (possibly abstract) base class. – Some programmer dude Jul 22 '21 at 12:55
  • 1
    _"I am not sure what to do"_ help us help you. Are you asking what an abstract type is? Are you asking why one can't be instantiated? Are you asking for sample libtins code that works? What is the question you are asking here? – Drew Dormann Jul 22 '21 at 13:00
  • `PDU` is declared inside a namespace `Tins`. Maybe you use Tins::PDU or you write `using namespace Tins;`at the at the head of your file. – MiniMik Jul 22 '21 at 13:00
  • @DrewDormann You know I need to fix whole situation to be able to dequeue. Right now, it doesn't compile because of the abstract class. – jeffbRTC Jul 22 '21 at 13:52
  • @Someprogrammerdude I did try pointers but the the queue lib use move so it didn't work.. – jeffbRTC Jul 22 '21 at 13:52
  • @MiniMik That didn't fix the problem too. – jeffbRTC Jul 22 '21 at 13:55
  • PDU is a polymorphic type, so you'll need some kind of pointer. Minimally you would have to change to: `moodycamel::ConcurrentQueue PacketQueue;` and `Tins::PDU *pdu;`, `bool callback(PDU *pdu)`, but then you'll have lifetime/ownership issues with the `PDU` objects -- so you should probably use a queue of `unique_ptr` (or some other kind of smart pointer) instead so that ownership of the PDU objects is clear. – Wyck Jul 22 '21 at 14:12
  • What is the type of `PacketQueue`? I'm going to assume it's something like `Queue` – Chad Jul 22 '21 at 15:01
  • @Chad Correct. Somehow GCC able to detect the type. I was not even using using namespace thingie. – jeffbRTC Jul 22 '21 at 15:57

1 Answers1

2
moodycamel::ConcurrentQueue<PDU> PacketQueue;

...

bool callback(PDU &pdu)
{
  PacketQueue.enqueue(pdu);

This cannot work correctly.

You received a reference to the base class of some concrete object. Enqueueing this by value causes object slicing - you're just copying the base-class subobject and discarding all the derived class state (and type information).

The fact that the compiler stopped you from dequeuing this abstract type is a handy coincidence, because otherwise you would have had to figure out why your successful enqueue/dequeue was producing garbage pdus.

I've attempted the pointer route ...

Well, you need some kind of indirection. You haven't shown the code you tried, but I think you forgot to change the type of the queue to match.

If you want to en/de-queue pointers, you need to:

  1. change your queue to be a queue of pointers
  2. clone (polymorphic deep-copy) the object to be enqueued (assuming the object whose reference you get, will be cleaned up after the callback returns)
  3. remember you're now responsible for destroying the cloned pdu after you dequeue it (or, better, just use unique_ptr in the first place)

The correct solution probably looks like this (completely untested) example:

// title case is conventionally used for types, not variables
// (of course, it would also be better to avoid globals ...)
moodycamel::ConcurrentQueue<std::unique_ptr<PDU>> packetQueue;

void worker()
{
    while(true) {
        std::unique_ptr<PDU> pdu;

        if(packetQueue.try_dequeue(pdu) == false) {
            continue;
        }

        // Do Work on *pdu
    }
}

bool callback(PDU &pdu)
{
  packetQueue.enqueue(std::unique_ptr<Tins::PDU>(pdu.clone()));

  return true;
}
jeffbRTC
  • 1,941
  • 10
  • 29
Useless
  • 64,155
  • 6
  • 88
  • 132
  • 1
    But what should I do at this stage to make this work correctly? – jeffbRTC Jul 22 '21 at 14:11
  • But where I find this `clone()` method of pdu. The pdu doesn't comes with that method.. – jeffbRTC Jul 22 '21 at 14:27
  • [O](https://github.com/mfontanini/libtins/blob/e90e377b73ba9d8181a9cb97586c7f1255d21eed/include/tins/pdu.h#L418) [RLY](http://libtins.github.io/tutorial/#pdus)? If you're using some version of libtins that lacks the `clone` method, perhaps you could share that information with us? – Useless Jul 22 '21 at 14:29
  • Found it, but for some reason Intellisense didn't show it up on options – jeffbRTC Jul 22 '21 at 14:36
  • Out of curiosity, Is there a way to avoid the copy? – jeffbRTC Jul 22 '21 at 14:37
  • There is typo on your code, `PacketQueue` should be `packetQueue` – jeffbRTC Jul 22 '21 at 14:38
  • Fixed, thanks. And try not to rely entirely on Intellisense when there's documentation and the code is available. It's a shortcut, not the gospel. – Useless Jul 22 '21 at 14:56
  • Noted. Any ideas on avoiding copy? – jeffbRTC Jul 22 '21 at 15:58
  • The only way to avoid the copy would be to take ownership of the original, but they way libtins passes by reference suggests this is not possible. You would need to look at how the library manages these objects, and possibly modify it so they can be adopted by the callback. – Useless Jul 22 '21 at 16:41
  • 1
    `packetQueue.enqueue(std::make_unique(pdu.clone()));` gives `error: invalid new-expression of abstract class type Tins::PDU { return unique_ptr<_Tp>(new _Tp(std::forward<_Args>(__args)...)); }` – jeffbRTC Jul 22 '21 at 18:03
  • so it seems happening again because we're trying to construct an abstract class called A from a B. But in my case, B is unknown so I can't fix this! Right? – jeffbRTC Jul 22 '21 at 18:13
  • 3
    So, `std::unique_ptr(pdu.clone())` works instead make_unqiue – jeffbRTC Jul 22 '21 at 19:57