My returnList[0] gets rewritten to @5'

Question

I am trying to return an array of strings and while I copy the strings something weird happens when it passes the 4th index. For example, when it loops through the first 3 times it is stored as "the" but then it sudden becomes rewritten but it writes the next index just fine[index 5]. Can you guys find anything wrong with it because I'm stumped.

#include <stdlib.h>
#include <stdio.h>
#include "hash.h"
#include <string.h>
#define MAX 200
#define TERMINATE "asdfghjkl"

int createTable(int numFiles, char** files, char** stopList)
{
  printf("stepped into create table\n");
  FILE* fp1;
  char oneWord[100];

  HashTable hash = InitializeTable(900000);
  int index = 2;

  while(numFiles >0) {
    fp1 = fopen(files[index++], "r");
    while(fscanf(fp1, "%s", oneWord)!=EOF){
      Insert(oneWord, hash, stopList);
    }
  numFiles--;
  }
  return 0;
} 

char** createStopList(char* stopL)
{
  FILE* fp1;
  fp1 = fopen(stopL, "r");
  char oneWord[100];
  int i = 0;  
  char* stopList[MAX];
  while(fscanf(fp1, "%s", oneWord)!=EOF){
    stopList[i] = (char*)malloc(sizeof(oneWord));
    strcpy(stopList[i++], oneWord);
  }
  stopList[i] = (char*)malloc(sizeof(char*));
  strcpy(stopList[i], TERMINATE);

  char** strings = stopList;
  char** returnList = malloc(sizeof(strings));
  i=0;
  while(strcmp(strings[i], TERMINATE)!=0){
    returnList[i] = malloc(sizeof(char*));
    strcpy(returnList[i], strings[i]);
    i++;
  }
  returnList[i] = (char*)malloc(sizeof(char*));
  strcpy(returnList[i], TERMINATE);
  return returnList;
}

int main(int argc, char** argv)
{
  printf("start of prg\n");
  char** stopList= createStopList(argv[1]);
  createTable(argc-2, argv, stopList);
  return 0;
} 

Answer 1

This code causes a buffer overflow:

#define TERMINATE "asdfghjkl"
// ...
returnList[i] = (char*)malloc(sizeof(char*));
strcpy(returnList[i], TERMINATE);

The length of TERMINATE is 10, but sizeof(char*) is probably less than 10.

To fix it:

returnList[i] = malloc( sizeof TERMINATE );
strcpy(returnList[i], TERMINATE);

Your comments suggest you used strdup instead (that function is not in Standard C, but it is commonly provided).

This is also completely fubar'd:

char** strings = stopList;
char** returnList = malloc(sizeof(strings));
// ...
returnList[i] = malloc(sizeof(char*));

sizeof(strings) is the same as sizeof(char **), which is probably 4 or 8, but you go on to write past the end of this array, as soon as i gets to 1! This is probably the cause of your symptoms.

I think perhaps you have a misconception about what sizeof does. It tells you how many bytes are used to store a variable (NOT how many bytes are at the location the variable is pointing to, if that variable is a pointer).

Presumably you meant:

returnList = malloc( (i+1) * sizeof *returnList );

which gives you enough pointers for indices returnList[0] through returnList[i].

The code after that is badly designed, you have unnecessary code duplication. Change the while loop to do...while, then the last iteration will copy TERMINATE for you without you having to write extra code for it.

Earlier on in that same function, this line is poor:

while(fscanf(fp1, "%s", oneWord)!=EOF){

You should prevent the input overflowing. Also you never check whether i exceeds MAX. And could make another improvement. Instead of copying TERMINATE into stopList, just save i, and write TERMINATE on the end of returnList.

Finally you seem to be pointlessly storing and copying your array instead of just dynamically allocating it in the first place. Oh, and your mallocs have warts.

Putting all of those changes together:

char **createStopList(char const *stopL)
{
    FILE* fp1;
    fp1 = fopen(stopL, "r");
    char oneWord[100];
    size_t i;
    char **stopList;

    if ( !fp1 )
        return NULL;

    stopList = malloc(MAX * sizeof *stopList);
    if ( !stopList )
         exit(EXIT_FAILURE);

    for (i = 0; i < MAX - 1 && fscanf(fp1, "%99s", oneWord) == 1; ++i)
    {
        stopList[i] = malloc( strlen(oneWord) + 1); 
        if ( !stopList[i] )
            exit(EXIT_FAILURE);
        strcpy(stopList[i], oneWord);
    }

    stopList[i] = malloc(sizeof TERMINATE);
    strcpy(returnList[i], TERMINATE);

// (optional) free entries you didn't use in the list
    stopList = realloc(stopList, (i+1) * sizeof *returnList);
    if ( !stopList )
        exit(EXIT_FAILURE);

    return stopList;
}