conflict of using strcpy , strcat in C?

Question

In the following code I'm trying to load a text file of words character by character then I'm trying to save each whole word in hash table (array of strings) but it seems that strcpy saves a whole word not a single char and I don't know why. Am I misusing strcpy and strcat?

# include <stdio.h>
# include <stdlib.h>
# include <string.h>
# include <ctype.h>
# include <stdbool.h>
bool load(const char* dictionary);

#define LENGTH 45


int main (int argc, char* argv[])
{
  char* dictionary = argv[1];
  load(dictionary);
  return 0;
}

bool load(const char* dictionary)
{
  int index = 0, words = 0, kk = 0;
  int lastl = 0, midl = 0;
  char word[LENGTH + 1];
  char *wholeword[1001];

  FILE* dic = fopen(dictionary, "r");
  if (dic == NULL)
  {
    printf("Could not open %s.\n", dictionary);
    return false;
  }

  for (int c = fgetc(dic); c != EOF; c = fgetc(dic))
  {
    // allow only alphabetical characters and apostrophes
    if (isalpha(c) || (c == '\'' && index > 0))
    {
      // append character to word
      word[index] = c;
      index++;

      // ignore alphabetical strings too long to be words
      if (index > LENGTH)
      {
        // consume remainder of alphabetical string
        while ((c = fgetc(dic)) != EOF && isalpha(c));
        // prepare for new word
        index = 0;
      }
    }

    // ignore words with numbers (like MS Word can)
    else if (isdigit(c))
    {
      // consume remainder of alphanumeric string
      while ((c = fgetc(dic)) != EOF && isalnum(c));

      // prepare for new word
      index = 0;
    }

    // we must have found a whole word
    else if (index > 0)
    {
      // terminate current word
      word[index] = '\0';
      lastl = index - 1;
      midl = (index - 1) % 3;
      words++;
      index = 0;

      int hashi = (word[0] + word[lastl]) * (word[midl] + 17) % 1000;

      wholeword[hashi] = (char*) malloc(sizeof(char) * (lastl + 2));

      strcpy(wholeword[hashi], &word[0]);  // ***

      for (kk = 1; kk <= lastl + 1; kk++)
      {
        strcat(wholeword[words], &word[kk]);
      }
    }
  }
  fclose(dic);
  return true;
}

It's hard to understand what your question is. The `strcpy` function, as its name suggests, copies a string. What is `wword`? — David Schwartz, Nov 01 '16 at 20:43
@DavidSchwartz wword is typo mistaken here (i edited it ) to be wholeword (array of strings ) , thanks — Mostafa Radwan, Nov 02 '16 at 09:48

score 2 · Accepted Answer · edited Nov 02 '16 at 04:06

2

Strcpy doesn't copy a single char, it copies all chars until the next null ('\0') byte. To copy a single char in your code try:

wholeword[hashi] = &word[0];

instead of:

strcpy(wholeword[hashi], &word[0]);

edited Nov 02 '16 at 04:06

Jonathan Leffler

730,956
141
904
1,278

answered Nov 01 '16 at 21:04

colorblind

158
2
7

Please don't put spaces before array subscripts — they bind very tight and should not be spaced like that. For example, in `&word [0]`, the precedence is `&(word [0]`) rather than `(&word) [0]`. – Jonathan Leffler Nov 02 '16 at 04:10

chqrlie · Answer 2 · 2016-11-02T10:10:41.800

Yes you are misusing strcpy and strcat: these functions copy a whole source string to the destination array (at the end of an existing string there for strcat).

The following lines:

  wholeword[hashi] = (char*) malloc(sizeof(char) * (lastl + 2));

  strcpy(wholeword[hashi], &word[0]);  // ***

  for (kk = 1; kk <= lastl + 1; kk++)
  {
    strcat(wholeword[words], &word[kk]);
  }
}

Can be replaced with a single call to

   wholeword[hashi] = strdup(word);

strdup() allocates the memory, copies the argument string to it and returns the pointer. It is available on all Posix systems, if you do not have it, use these 2 lines:

  wholeword[hashi] = malloc(lastl + 2);
  strcpy(wholeword[hashi], word);

Notes:

you assume your hash to be perfect, without collisions. As currently coded, a collision causes the previous word to be removed from the dictionary and its corresponding memory to be lost.
the dictionary char *wholeword[1001]; is a local variable in the load function. It is uninitialized, so there is no way to know if an entry is a valid pointer to a word. It should be allocated, initialized to NULL and returned to the caller.

conflict of using strcpy , strcat in C?

2 Answers2