0

here is my code

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

#define STRING_LENGTH 20
#define MAX 30

int read_string(char string[], int n);
int compare(const void*a, const void*b);

int main(){
    int i;

    char* word_list[30];
    char word[STRING_LENGTH + 1];

    for (i = 0; ; i++){
       printf("\nEnter a word.\n");
       read_string(word, STRING_LENGTH);

       if (word[0] == '\0')
           break;
       word_list[i] = (char *)malloc(STRING_LENGTH + 1);
       //free(word_list);
       strcpy(word_list[i], word);
    }

    int length = sizeof(word_list)/sizeof(char*);

    int j;

    qsort(word,length, sizeof(char*), compare);
    for (j = 0; word_list[j] != '\0'; j++)
        printf("%s\n", word_list[j]);

    return 0;
}

int compare(const void*element1, const void *element2){
    const char *string1 = *(const char**)element1;
    const char *string2 = *(const char**)element2;

    return strcmp(string1,string2);
}

int read_string(char string[], int n){
    int ch, i = 0;

    while ((ch = getchar()) != '\n')
        if (i < n)
            string[i++] = ch;
    string[i] = '\0';

    return i;
}

My program is supposed to read in strings via the read_string function and then strcpy is used to place them as elements of an array of pointers, then the names are sorted alphabetically. It compiles and reads i my input but it crashes once it gets to qsort(). I know qsort() is causing the problem but I don't know why. Any help would be very appreciated.

WedaPashi
  • 3,561
  • 26
  • 42
Flower
  • 381
  • 1
  • 6
  • 17
  • I have to allocate memory for each element so that a string can be copied into it. The break from the loop happens when the user presses enter after entering nothing. That part causes me no issues – Flower Jul 09 '15 at 05:21
  • 1
    Yes I noticed I did not see that it was an array of char* – Cyclonecode Jul 09 '15 at 05:22
  • any advice about why qsort is causing the problem, it crashes once it gets to that line. – Flower Jul 09 '15 at 05:25
  • 3
    "That part causes me no issues " - guess again. `int length = sizeof(word_list)/sizeof(char*);` is wrong unless you happened to full populate all `30` slots. otherwise you're sorting `30-i` indeterminate pointers along with whatever pointers you *did* populate. As soon as those junk-pointers make it to your comparator and you invoke `strcmp` (which dereferences them), you've invoked *undefined behavior*. – WhozCraig Jul 09 '15 at 05:29
  • 2
    Problem is you give `qsort()` the FULL list of pointers, no matter how many of them actually point to valid data. Also, your code happily writes past the end of `word_list` if the user happens to enter more words. Never write code like this ... –  Jul 09 '15 at 05:31
  • @WhozCraig could i use free() to fix the junk pointers problem? – Flower Jul 09 '15 at 05:41
  • 2
    *They're not used*. Sort the list by telling `qsort` how many actual items to sort (the number of strings you read). Print the results using the number of strings you read. And if you were serious about your last question, it may behoove you to invest in [a decent book or two on C](https://stackoverflow.com/questions/562303/the-definitive-c-book-guide-and-list) – WhozCraig Jul 09 '15 at 05:44
  • another way around this is to dynamically allocate space for each element in the word_list array right? without first declaring it with a const size? How would i do this without getting a possible malloc error? – Flower Jul 09 '15 at 05:50

2 Answers2

3

There are a number of issues as pointed out in the comments. The primary being you are calling qsort with the incorrect pointer, and incorrect number of members (30) instead of the number of strings read. A correction would be:

qsort (word_list, i, sizeof (char *), compare);

While you can use a sentinel to stop the print after qsort completes, why? You already know how many strings you read in i. So simply:

for (j = 0; j < i; j++)
    printf ("%s\n", word_list[j]);

As an aside, it would be useful to be able to stop input after any number of strings in read_string. Allowing for an keyboard generated EOF to signal end of input by ctrl+d (ctrl+z on windows) will work. E.g.:

while ((ch = getchar ()) != '\n' && ch != EOF)
David C. Rankin
  • 81,885
  • 6
  • 58
  • 85
  • this for loop works well now but as i stated in another comment my final string output is a random set of characters – Flower Jul 09 '15 at 06:01
  • 1
    Hmm.. I compiled, debugged and ran the code with the above changes and found no issues. You need to insure you are not modifying `i` after your read loop or you will lose the number of strings read. It is generally better to declare a specific variable say `numstr` and following your input loop set `numstr = i;` since you may be tempted to use `i` again as a loop counter. The only way you will get random chars is a `read beyond` the number of strings in `word_list`. Insure that is not taking place. – David C. Rankin Jul 09 '15 at 06:04
  • I just had <= in my for loop instead of <. problem resolved. Thankyou much1 – Flower Jul 09 '15 at 06:06
2

Notice that it is word_list and not word you would like to sort.

int main() {
  int i;

  char* word_list[30];
  char word[STRING_LENGTH + 1];

  for(i = 0; ; i++) {
    printf("\nEnter a word.\n");
    read_string(word, STRING_LENGTH);

    if(word[0] == '\0')
      break;

    word_list[i] = (char *)malloc(STRING_LENGTH + 1);
    strcpy(word_list[i], word);
  }

  // call qsort with the number of items in the wordlist
  qsort(word_list, i, sizeof(char*), compare);

}


int compare(const void*element1, const void *element2){
  const char **string1 = (const char**)element1;
  const char **string2 = (const char**)element2;

  return strcmp(*string1,*string2);
}

You should also call set length to the number of items in word_list before calling qsort() not to the number of characters in a word.

For instance if you have read in two words from stdin then you call qsort() like this qsort(word_list, 2, sizeof(char*), compare);

Perhaps I also need to mention that you should free the memory allocated by malloc() when you have finished using the word list.

Cyclonecode
  • 29,115
  • 11
  • 72
  • 93
  • thanks for your help. It did not crash however when i entered "foo","poo" and "boo", it printed "foo" "poo" "strange set of characters". – Flower Jul 09 '15 at 06:00
  • 1
    @Flower - You need to call `qsort()` with the number of words located inside the list. – Cyclonecode Jul 09 '15 at 06:01
  • Actually it printed them in alphabetical order, i just overlooked my for loop. my condition used <= instead of <. thanks again for your help. – Flower Jul 09 '15 at 06:03
  • @Flower - Just glad I could help and that you fixed it =) – Cyclonecode Jul 09 '15 at 07:05