2013-06-11 2 views
0

J'ai des problèmes à faire fonctionner Code. Il analyse une entrée d'utilisateurs dans un char * [] et le renvoie. Cependant, la commande char * [] n'accepte aucune valeur et reste remplie de NULL ... que se passe-t-il ici?Fill char * [] from strtok

void* setCommands(int length){   
    char copy[strlen(commandline)]; //commandline is a char* read with gets(); 
    strcpy(copy, commandline); 
    char* commands[length]; 
    for (int x=0; x<length; x++) 
     commands[x] = "\0"; 
    int i = 0; 
    char* temp; 
    temp = strtok (copy, " \t"); 
    while (temp != NULL){ 
     commands[i] = temp; //doesnt work here.. commands still filled with NULL afterwards 
     i++; 
     printf("word:%s\n", temp); 
     temp = strtok (NULL, " \t"); 
    } 
    commands[i] = NULL; 
    for (int u=0; u<length; u++) 
     printf("%s ", commands[i]); 
    printf("\n"); 
    return *commands; 
} 

Vous pouvez supposer que commandline! = NULL, longueur! = 0

+0

'char * commandes [longueur];' et 'commandes [x] =" \ 0 ";' - qu'est-ce que vous essayez de faire ici? Si vous voulez écrire sur un pointeur, commencez par lui allouer de la mémoire en utilisant 'malloc'. Au moment où votre déclaration dit 'déclare des commandes comme un tableau de longueur égale à' length 'de pointeur sur char'. Ou déclarez simplement un tableau de caractères 2D pour ne pas vous soucier de la gestion manuelle de la mémoire. – Nobilis

+0

La boucle for était une tentative pour initialiser les éléments du tableau, comme je pensais que c'était le problème ... eh bien ce n'était pas le cas. –

Répondre

0
commands[i] = NULL; 
for (int u=0; u<length; u++) 
    printf("%s ", commands[i]); 

Prenez un très bon coup d'oeil à ce code. Il utilise u comme variable de contrôle de boucle mais imprime l'élément en fonction de i. Par conséquent, étant donné que vous avez défini commands[i] sur NULL dans la ligne avant la boucle, vous obtiendrez simplement une série de valeurs NULL.

Utilisez commands[u] dans la boucle plutôt que commands[i].


En plus de cela:

void* setCommands(int length){   
    char* commands[length]; 
    : 
    return *commands; 
} 

ne retourneront un pointeur, celui du premier jeton, pas celui au tableau de pointeurs jeton. Vous ne pouvez pas retourner les adresses de variables locales qui sortent de la portée (eh bien, vous pouvez peut, mais cela peut ne pas fonctionner).

Et, dans tous les cas, puisque ce pointeur pointe très probablement encore une autre variable locale (quelque part dans copy), il est également invalide.

Si vous souhaitez renvoyer des blocs de mémoire à partir de fonctions, vous devez utiliser malloc, dans ce cas à la fois pour le tableau de pointeurs et pour les chaînes elles-mêmes.

+0

La boucle for était une tentative pour initialiser les éléments du tableau, comme je pensais que c'était le problème ... eh bien ce n'était pas le cas. –

+0

Quoi? Votre commentaire sur la modification de 'temp' et sur le fait que toutes les valeurs pointent vers une valeur finale est incorrect. – paddy

+0

J'ai essayé les commandes [i] = strdup (temp); Cependant, il ne résout pas les problèmes ... toujours NULL partout –

0

Vous avez un certain nombre de problèmes ... Votre programme affichera un comportement indéfini actuellement, donc tant que vous n'aborderez pas les problèmes, vous ne pouvez pas espérer prédire ce qui se passe. Commençons.

La chaîne suivante est un caractère trop court. Vous avez oublié d'inclure un caractère pour le terminateur de chaîne ('\0'). Cela entraînera un dépassement de la mémoire tampon pendant la segmentation, ce qui pourrait être en partie responsable du comportement que vous observez.

char copy[strlen(commandline)]; // You need to add 1 
strcpy(copy, commandline); 

La partie suivante est votre valeur de retour, mais il s'agit d'un tableau temporaire (tableau local). Vous n'êtes pas autorisé à retourner cela. Vous devriez l'allouer à la place.

// Don't do this: 
char* commands[length]; 
for (int x=0; x<length; x++) 
    commands[x] = "\0";   // And this is not the right way to zero a pointer 

// Do this instead (calloc will zero your elements): 
char ** commands = calloc(length, sizeof(char*)); 

Il est possible pour la boucle de tokenising de dépassement de votre tampon parce que vous ne jamais vérifier pour length, vous devez donc ajouter un test:

while(temp != NULL && i < length) 

Et à cause de ce qui précède, vous ne pouvez pas mettre aveuglément commands[i] à NULL après la boucle. Testez le i < length ou ne le configurez pas (vous avez d'abord mis le tableau au zéro).

Passons maintenant à la valeur de retour.Actuellement, vous avez ceci:

return *commands; 

qui renvoie un pointeur vers le premier jeton dans votre chaîne temporaire (copy). Tout d'abord, il semble que vous vouliez retourner un tableau de jetons, pas seulement le premier jeton. Deuxièmement, vous ne pouvez pas retourner une chaîne temporaire. Donc, je pense que vous vouliez dire ceci:

return commands; 

Maintenant, pour faire face à ces cordes ... Il y a un moyen facile et une façon intelligente. La méthode facile a déjà été suggérée: vous appelez le strdup sur chaque jeton avant de les mettre en mémoire. La partie ennuyante de ceci est que quand vous nettoyez cette mémoire, vous devez passer par le tableau et libérer chaque jeton individuel.

Au lieu de cela, nous allons faire tout en un seul coup, en affectant le réseau et le stockage de chaîne dans un appel:

char **commands = malloc(length * sizeof(char*) + strlen(commandline) + 1); 
char *copy = (char*)(commands + length); 
strcpy(copy, commandline); 

La seule chose que je ne l'ai pas ci-dessus est égal à zéro le tableau. Vous pouvez le faire après la boucle de tokenising, par la mise à zéro juste les valeurs restantes:

while(i < length) commands[i++] = NULL; 

Maintenant, lorsque vous revenez commands, vous renvoie un tableau de jetons qui contient également son propre stockage jeton. Pour libérer le tableau et toutes les chaînes qu'il contient, vous venez le faire:

free(commands); 

Mettre tous ensemble:

void* setCommands(int length) 
{ 
    // Create array and string storage in one memory block. 
    char **commands = malloc(length * sizeof(char*) + strlen(commandline) + 1); 
    if(commands == NULL) return NULL; 
    char *copy = (char*)(commands + length); 
    strcpy(copy, commandline); 

    // Tokenise commands 
    int i = 0; 
    char *temp = strtok(copy, " \t"); 

    while(temp != NULL && i < length) 
    { 
     commands[i++] = temp; 
     temp = strtok(NULL, " \t"); 
    } 

    // Zero any unused tokens 
    while(i < length) commands[i++] = NULL; 

    return commands; 
}