Return pointer on an array of lists?

Question

I try to write a little shell. For this i want to implement a pipe function for numerous pipes. For this i try to break up a list of words at pipeflags in smaller lists, to generate argument-arrays for execvp() an link them later per pipes.

I use this code for generating the list array:

struct shellvalue* list2listarray(struct shellvalue* values, unsigned int* pipecount) {
struct shellvalue* orderarray = (struct shellvalue*) malloc((pipecount[0] + 1) * sizeof(struct shellvalue));
struct shellvalue* tmp;
tmp = *(&orderarray);
struct shellvalue* currentlist;
int i = 0;
int j = 0;
while (i <= *pipecount) {
    j = 0;
    while (values && !(values->ispipe)) {
        addtoken(values->word, j, &orderarray, &currentlist);
        values = values->next;
        j++;
    }if(values){
        values = values->next;
        orderarray++;
    }
    i++;
}
tmp ++;
return tmp;}

The addtocken method is implemented this way:

void addtoken(char* word, int counter, struct shellvalue** start,
    struct shellvalue** current) {
if (counter == 0) {
    size_t structsize = sizeof(struct shellvalue);
    struct shellvalue* tmp = (struct shellvalue*) malloc(structsize);
    tmp->word = malloc(strlen(word) + 1);
    strcpy(tmp->word, word);
    *start = tmp;
    (*start)->next = NULL;
    *current = *start;
} else {
    struct shellvalue* new = (struct shellvalue*) malloc(
            sizeof(struct shellvalue));
    new->word = malloc(strlen(word) + 1);
    strcpy(new->word, word);
    new->next = NULL;
    (*current)->next = new;
    *current = new;
}}

My problem is a classic pointer problem. I cant manage to give back a working pointer with my list2listarray-Method on my list-array, so i can easily switch through my lists. Can someone see the problem? Thank you very much for your time.

You know that the dereference operator `*` and the address-of operator `&` complements each other, and does, basically, the opposite things. That means the expression `*(&orderarray)` is the same as just `orderarray`. — Some programmer dude, May 04 '14 at 08:34
Removing the casts, replacing the while-loops by for-loops and removing excessive parentheses would probably yield a more readable program. Also: I don't trust the recursive call , and `(*current)->next = new; *current = new;` looks wrong, too. the offset-1 indexing in `while (i <= *pipecount) {` looks wrong, too — wildplasser, May 04 '14 at 08:38

score 1 · Accepted Answer · edited May 23 '17 at 12:20

In the function list2listarray you pass the variable orderarray by reference (or as close as it can be in C) to the addtoken function, so when you do *start = tmp in the addtoken function you actually change the pointer in the list2listarray function as well, loosing your initial pointer you allocated there.

You also have a problem in list2listarray with your tmp++ as that makes you loose the original pointer as well.

There is no need to pass this pointer "by reference" to the addtoken function, or to allocate there in the first place.

There are probably more problems related to your pointer use in the shown code, most likely leading to undefined behavior. I recommend you turn on more warnings by the compiler, as those are a good way to find possible places where you have undefined behavior, but don't just add casting as a way to shut the warnings up as it probably don't solve the actual root cause. And talking about casting, don't cast the return of malloc.

Return pointer on an array of lists?

1 Answers1