1

we are implementing a priority queue for generic type of datas in C. We think the assignments between pointers etc. are made right, but we don't get how to print at the end the int value of "element". Can you help me?

#include <stdio.h>
#include <stdlib.h>

struct _pqElement{
  void** data;
  void** priority;
};

pqElement* pqElement_new(void* data, void* priority){
  pqElement* result = (pqElement*) malloc (sizeof(pqElement));
  result->data=&data;
  result->priority=&priority;
  return result;
}

static int* new_int(int value){
 int *elem=(int*)malloc(sizeof(int));
 *elem=value;
 return elem;
}

int main(int argc, char const *argv[]){
  pqElement * element = pqElement_new(new_int(1), new_int(85));
  printf("%d-%d", element->data, element->priority);
}
Paul Floyd
  • 5,530
  • 5
  • 29
  • 43
palnic
  • 386
  • 1
  • 4
  • 13
  • "We think the assignments between pointers etc. are made right," No, they are not: `result->data=&data;` you assign the address of your local copy of the pointer. After you leave the function, this address is invalid. Use `result->data=data;` instead. Same for `priority` – Gerhardh Jun 07 '18 at 10:15
  • Why would you want to store pointers to pointers at all? Why would you want to use a pointer (or even pointer to pointer) for the priority? – Gerhardh Jun 07 '18 at 10:16
  • Because we don't know what type of data we are going to store in the queue. That's why in the struct we have a void** pointer to store the pointer given by "new_int". However I fixed result->data=data, but still do you know how to print out the integer value which is pointed by it? – palnic Jun 07 '18 at 10:20
  • What's wrong with a `void*` instead? It can point to any data type. – Gerhardh Jun 07 '18 at 10:21
  • In fact, a `void*` can point at anything, but a `void**` should only point at a `void*` object, so `void*` is more general. – aschepler Jun 07 '18 at 10:22
  • @Gerhardh, aschepler [re](https://stackoverflow.com/questions/50738225/how-to-print-a-double-pointer-value-in-c#comment88485090_50738225). `void*` does not point to any type. `void*` may be insufficient to completely copy a function pointer. It is big enough to store object pointers. C lacks a universal pointer primitive. A close construct is `union { void *vp; int (fp)(); }`. – chux - Reinstate Monica Jun 07 '18 at 11:37
  • palnic, "we don't know what type of data we are going to store in the queue" --> what do you want to do if the data does not fit in a `void*`? – chux - Reinstate Monica Jun 07 '18 at 11:40

3 Answers3

1

Well the code as it is does not even compile, since the pqElement type has never been define, but only the _pqElement struct has been defined.

Also you are using %d in the printf, but the parameter you are passing is void**, so you need to cast the value.

These changes should make the trick:

#include <stdio.h>
#include <stdlib.h>

typedef struct _pqElement{
    void** data;
    void** priority;
} pqElement;

pqElement* pqElement_new(void* data, void* priority){
    static pqElement* result;
    result = (pqElement*) malloc (sizeof(pqElement));
    result->data=&data;
    result->priority=&priority;
    return result;
}

int* new_int(int value){
    static int *elem;
    elem = (int*)malloc(sizeof(int));
    *elem=value;
    return elem;
}

int main(int argc, char const *argv[]){
    pqElement *element = pqElement_new(new_int(1), new_int(85));
    printf("%d-%d\n", **((int**)(element->data)), **((int**)(element->priority)));
    //need to free the memory allocated with the malloc, otherwise there is a possibility of memory leakage!
}

This will only print the first element, but you can point to the following elements by using an offset.

NOTE: As I reported as a comment in the code, you need to free the memory allocated using the malloc, otherwise you have potential memory leakage!

  • 1
    The issue with using non-static addresses in `pqElement_new` which get invalid after returning is also present in your code. – Gerhardh Jun 07 '18 at 10:22
  • Is the added `static` keyword intented to solve the problem? – Gerhardh Jun 07 '18 at 10:42
  • Sorry I actually used the static keyword the wrong way. I updated the answer. Adding the static keyword ensures that the variable is stored in the static part of the memory and not the stack. The previous answer was working but had a safety issue, since the stack variables of the function are de-referenced when the function terminates and the pointer may be overwritten. You can find some details on memory division here: https://stackoverflow.com/questions/408670/stack-static-and-heap-in-c?utm_medium=organic&utm_source=google_rich_qa&utm_campaign=google_rich_qa – Alexander James Pane Jun 07 '18 at 12:32
  • The usage of `data` and `priority` are the problem, not `element`. – Gerhardh Jun 07 '18 at 12:36
1

You don't need two levels of pointers.

#include <stdio.h>
#include <stdlib.h>

struct pqElement{
  void *data;
  void *priority;
};

struct pqElement* pqElement_new(void* data, void *priority)
{
  struct pqElement* result = malloc(sizeof(struct pqElement));
  result->data = data;
  result->priority = priority;
  return result;
}

static int* new_int(int value)
{
  int *elem = malloc(sizeof(int));
  *elem=value;
  return elem;
}

int main(int argc, char const *argv[])
{
  struct pqElement *element = pqElement_new(new_int(1), new_int(85));
  printf("%d-%d", *(int*)element->data, *(int*)element->priority);
}

Finally printing the value requires to cast the pointer in a proper way, as Alexander Pane already mentioned.

Using different type for priority as well will make the queue a bit less generic. You need to provide different functions for sorting, printing, etc.

Gerhardh
  • 11,688
  • 4
  • 17
  • 39
  • We put "priority" as "void*" cause we also need to use the same code to implements the Prim's algorithm which have the weight as float value. – palnic Jun 07 '18 at 10:34
-1

Thank you, guys. I need the code works also with strings so I did like this:

static char* new_string(char* value){
  char* elem= malloc (sizeof(char));
  strcpy(elem, value);
  return elem
}

Printing out like this:

 int main(int argc, char const *argv[]){
   struct pqElement *element = pqElement_new(new_string("Hello"), 85);
   printf("%s-%d", *(char*)element->data, element->priority);
 }
palnic
  • 386
  • 1
  • 4
  • 13
  • 1
    You need to allocate `strlen(value)+1` bytes. Not only 1. For printing you need to remove the leading `*`. – Gerhardh Jun 07 '18 at 10:47
  • 1
    BTW: This is not an answer and you should not post it as an answer. Instead add a new question or extend your question. – Gerhardh Jun 07 '18 at 10:54