Arbitrary-strings using getline

Question

So basically what my program did before i had to change it so that it would accept arbitrary values, was to take x-amount of words and the size of the words would also be arbitrary. (both are user inputted). I did this via a multiArray. Then sorted according to alphabetical-order.

I'm just going to put it out there as my code is shit and I'm very unfamiliar with the usage of arbitrary-strings and pointers. I've read up on it in the manual but the concept needs to sink in a little bit first i believe. Anyhow, I get the error: "Abort trap: 6" when i run the program. Could anyone please help me fix this problem so that i can see how the code would look like if it was actually working, i think that would help me understand both pointers and allocating memory a lot better. Forever in debt if you do.

Current code:

#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#define MAX_LENGTH 10
int main(){ //8

  char *name;
  char tname[] = {0};
  char temp[] = {0};
  int i=0, j=0, n=0;
  ssize_t bytes_read;
  size_t bytes_number;


  printf("Enter the amount of words you want to input: ");
  scanf("%d", &n);
  printf("Enter %d words: ",n);

  bytes_number = MAX_LENGTH;
  name = (char *) malloc (bytes_number+ 1);
  bytes_number = 0;
  bytes_read = getline(&name, &bytes_number, stdin);

  if (bytes_read == -1){
    puts("ERROR!");
    free(name);
  }

  for (i = 0; i < n; i++){
      strcpy(&tname[i], &name[i]);
  }
  for (i = 0; i < n - 1 ; i++){
      for ( j = i + 1; j < n; j++){
          if (strcmp(&name[i], &name[j]) > 0){
              strcpy(temp, &name[i]);
              strcpy(&name[i], &name[j]);
              strcpy(&name[j], temp);
          }
      }
  }
  printf("\n------------------------------------------\n");
  printf("%-3s %4s %11s\n", "Input","|", "Output");
  printf("------------------------------------------\n");
  for (i = 0; i < n; i++)
  {
      printf("%s\t\t%s\n", &tname[i], &name[i]);
  }
  printf("------------------------------------------\n");
  }

Just `puts("ERROR!");` and continue is not proper error handling. Why do you cast `malloc()`, does the compiler complain? And also, `char tname[] = {0};` doesn't look like what you want. — Iharob Al Asimi, Dec 19 '15 at 16:06
The program compiles fine, but after i've went through the first input-stage, the program crashes. Do you have any solutions in mind? — Joel, Dec 19 '15 at 16:08
Note: Compiles fine without `-Wall` is possibly not really *fine*. — Iharob Al Asimi, Dec 19 '15 at 16:08
@Joel You have to redesign your program. You need to store `n` words. But you need to consider: 1. a word cannot be longer than 10 characters, that's the limit you defined. - 2. you try to copy each word into the same variable (see answer of iharob); that's not what you want. - 3. you need to learn about arrays in C too, not only pointers. — Ely, Dec 19 '15 at 16:19

Iharob Al Asimi · Accepted Answer · 2015-12-19T16:41:25.687

This

strcpy(&tname[i], &name[i]);

is completely wrong, if you just want to copy all the characters, then it's just

strcpy(tname, name);

which is equivalent to

for (size_t i = 0 ; name[i] != '\0' ; ++i)
    tname[i] = name[i];

using strcpy(&tname[i], &name[i]) is wrong because it will copy all the bytes from name until '\0' is found, at every loop starting at the i-th character.

But this will fail again because tname is does not have room, it's an array with just one element.

Since you want to sort the strings, you DO NOT NEED TO COPY them. Just swap the pointers. Also

char temp[] = {0};

only allocates 1 character, thus

strcpy(temp, name);

will invoke Undefined Behavior.

Try this, maybe it's what you need

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

int
main(void)
{
    char **words;
    char *temp;
    int word_count;
    int actual_count;
    char *word;
    size_t length;
    int result;

    printf("Enter the amount of words you want to input: ");
    if (scanf("%d%*c", &word_count) != 1)
        return -1; // Input error
    printf("Enter '%d' words:\n", word_count);

    words = NULL;
    word = NULL;
    result = -1;
    actual_count = 0;
    length = 0;
    for (int i = 0 ; i < word_count ; ++i)
    {
        char **pointer;

        printf("Word(%d) > ", i + 1);

        if ((length = getline(&word, &length, stdin)) <= 0)
            goto cleanup;
        // Grow the array of words
        pointer = realloc(words, (i + 1) * sizeof(*pointer));
        if (pointer == NULL)
            goto cleanup; // Memory Exhausted
        // Now it's safe to overwrite `words'
        words = pointer;

        words[i] = malloc(length);
        if (words[i] == NULL)
            goto cleanup; // Memory Exhausted
        memcpy(words[i], word, length);
        words[i][length - 1] = '\0'; // Replace '\n' with '\0'
        actual_count += 1;
    }

    printf("Input : ");
    for (int i = 0 ; i < actual_count ; ++i)
        printf("%s\t", words[i]);
    printf("\n");

    for (int i = 0; i < actual_count - 1 ; i++)
    {
        for (int j = i + 1 ; j < actual_count ; ++j)
        {
            if (strcmp(words[i], words[j]) <= 0)
                continue;
            temp = words[i];
            words[i] = words[j];
            words[j] = temp;
        }
    }

    printf("Output: ");
    for (int i = 0 ; i < actual_count ; ++i)
        printf("%s\t", words[i]);
    printf("\n");

    result = 0;
cleanup:
    free(word);
    for (int i = 0; i < actual_count ; i++)
        free(words[i]);
    free(words);

    return result;
}

Note: This would consider an empty word (made completely of white space characters), as a valid word.

hmm. the compiler doesn't complain, but i can see the logic behind why it wouldnt work. As i said, i'm terrible at pointers... Thanks for your feedback tho. Do you have any other solutions to my problem as I'm sure it lies within the allocation. — Joel, Dec 19 '15 at 16:10
The compiler doesn't complain because it's valid syntax, but it's incorrect. — Iharob Al Asimi, Dec 19 '15 at 16:11
i see. I tried changing my strcpy function and sorting to what you suggested. Still no luck. The program is supposed to sort the words using bubble-sorting and put A infront of B etc. — Joel, Dec 19 '15 at 16:14
Don't i need to use arrays for this? How else would i be able to copy an entire word? I just need the function to assign arbitrary-length strings using user-input. ( getline() in this case). — Joel, Dec 19 '15 at 16:18
The logic in your program is not clear at all, what are you trying to do? — Iharob Al Asimi, Dec 19 '15 at 16:20
@Joel To copy "*arbitrary length string*" you can use `strdup()` in my example I don't, because if it's already known the length of the string, using `malloc()` + `memcpy()` is more efficient in this case. — Iharob Al Asimi, Dec 19 '15 at 16:48
Thank you so much. I will be sure to read up on what these things do in the manual. If you want i can give you the original working code without my attempt to create arbitrary strings. Might give you a better idea of my thoughtprocess from the working program -> this attempt i posted. :) Thanks anyway! You are golden! — Joel, Dec 19 '15 at 17:21

Arbitrary-strings using getline

1 Answers1