Wrong results being printed when dealing with arrays - C90

Question

The code:

#include <stdio.h>
#include <string.h>
#include <ctype.h>
#define W 5
#define N 10

/*Thanks to H.S. (stack overflow) for the  remove_consecutive_characters function */

void reverse_letters_and_matrix(char matrix[W][N], int max);
void convert_tolower(char matrix[W][N]);
void remove_consecutive_characters(char* pstr);

int main()
{    
  int j = 0;
  int i = 0;
  char words[W][N];
  char test[W][N];
  char endword[N] = "end";
  char newword[N];
  char matrix1[N] = { "Hello" };
  while (scanf("%9s", test), strcmp(test, endword))
  {
    strcpy(words[i++], test);

    j++;
    if (j == W)
    {
      break;
    }
  }

  for (i = 0; i < j - 1; i++)
  {
    {
      strcpy(words[i], words[i + 1]);
    }
  }

  reverse_letters_and_matrix(words, j);

  for (i = 0; i < j; i++)
  {
    remove_consecutive_characters(words[i]);
  }

  convert_tolower(words);

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

  printf("End of program");
  return 0;
}


void reverse_letters_and_matrix(char x[W][N], int max)
{
  int i, j, n;

  for (i = 0; i < max; i++)
  {
    for (j = 0, n = strlen(x[i]); j < n / 2; j++)
    {
      char c = x[i][j];
      x[i][j] = x[i][n - j - 1];
      x[i][n - j - 1] = c;
    }
  }

  for (i = 0; i < max / 2; i++)
  {
    char temp[N];
    strcpy(temp, x[i]);
    strcpy(x[i], x[max - i - 1]);
    strcpy(x[max - i - 1], temp);
  }

  return;
}


void remove_consecutive_characters(char* pstr)
{
  unsigned int i;
  if (pstr == NULL)
  {
    printf("Wrong input...\n");
    return;
  }

  char* p = pstr;
  int skip_letter = 0;

  for (i = 0; pstr[i]; ++i)
  {
    if ((tolower(pstr[i]) == tolower(pstr[i + 1])))
    {
      skip_letter = 1;
      continue;
    }

    if (skip_letter)
    {
      skip_letter = 0;
    }

    else
    {
      *p++ = pstr[i];
    }
  }

  *p = '\0';
}


void convert_tolower(char matrix[W][N])
{
  int i;
  int j;
  for (i = 0; i < W; i++)
  {
    for (j = 0; j < N; j++)
    {
      matrix[i][j] = tolower(matrix[i][j]);
    }
  }
  return;
}

The problem: I want this program to take 5 words at most as an input.Then reverse the words,reverse the way the are printed and remove consecutive repeated characters in each word. For example,if i have as an input

This
isss
AAAa
Testtt
end 

I should get

set
a
i
siht

But when I run the code I get

tset
tset


i

How can I fix that?.. I have tried in main function change the W's I had with j's but I guess that just made things worse..Also I am a newbie and do not know how to use the debugger..

Answer 1

With a good set of warnings enabled (-Wall -Wextra), my compiler lets me know that in each of

scanf("%9s", test)
strcmp(test, endword)
strcpy(words[i++], test)

the variable test has the wrong type for these functions, which expect char * or (const char *). test is of type char [N][M], which when passed to a function will decay to char (*)[M].

To fix, test should simply be char test[N];. That said, the intermediate buffer is unnecessary, as elements of words can be used directly.

Additionally, matrix1 and newword are unused.

---

The next issue is

for (i = 0; i < j - 1; i++)
{
  {
    strcpy(words[i], words[i + 1]);
  }
}

which is overwriting data:

This
isss
AAAa
Testtt

becomes

isss
AAAa
Testtt
Testtt

This should simply be removed as its purpose is unclear.

---

remove_consecutive_characters always operates in terms of lowercase characters. "AAAa" and "aaaa" are indistinguishable, and as such either one will result in an empty string.

---

Finally, by always looping to the maximal bounds, covert_to_lower accesses parts of words (in function as matrix) that were never initialized.

It would be better if convert_to_lower to deal with a single string at a time, and to separate the reverse_letters_and_matrix into its component code.

A refactored program

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

#define W 5
#define N 10

void reverse_matrix(char matrix[W][N], size_t max);
void reverse_word(char *string);
void convert_tolower(char *string);
void remove_consecutive_characters(char *string);

int main(void)
{
    size_t i = 0;
    size_t length = 0;
    char words[W][N];
    char endword[N] = "end";

    for (; length < W; length++)
        if (1 != scanf("%9s", words[length]) || 0 == strcmp(endword, words[length]))
            break;

    reverse_matrix(words, length);

    for (; i < length; i++) {
        reverse_word(words[i]);
        remove_consecutive_characters(words[i]);
        convert_tolower(words[i]);

        printf("%s\n", words[i]);
    }

    printf("End of program");

    return 0;
}


void reverse_matrix(char matrix[W][N], size_t max)
{
    size_t i = 0;

    for (; i < max / 2; i++) {
        char temp[N];

        strcpy(temp, matrix[i]);
        strcpy(matrix[i], matrix[max - i - 1]);
        strcpy(matrix[max - i - 1], temp);
    }
}

void reverse_word(char *string)
{
    size_t i = 0;
    size_t len = strlen(string);

    for (; i < len / 2; i++) {
        char c = string[i];
        string[i] = string[len - i - 1];
        string[len - i - 1] = c;
    }
}

void remove_consecutive_characters(char *string)
{
    int skip = 0;
    char *p = string;

    for (; *string; string++) {
        if (string[0] == string[1])
            skip = 1;
        else if (skip)
            skip = 0;
        else
            *p++ = *string;
    }

    *p = 0;
}

void convert_tolower(char *string)
{
    for (; *string; string++)
        *string = tolower((unsigned char) *string);
}

that does produce

set
a
i
siht

for the given input.