CODEWITHSUNDEEP
carrayspointerstokenize
user3444847
user3444847

Reputation: 65

Output problems in c

I have a problem with output of the make[] array is giving me. So first I tokenize the file called Carss.txt, only grabbing the third & fifth column, and store the result in make[] array and year[] array respectively.

I tested out the make[] array to see if it would output the results I wanted. It did inside the while(fgets) loop, but when I tested it on the outside of while(fgets), it's repeating.

I don't understand what's the problem. My program and the text file is provided below.

Output of make array inside while(fgets) loop: (what I wanted)

FORD
FORD
JAYCO

Output of make array outside while(fgets) loop:(Not what I wanted)

JAYCO
JAYCO
JAYCO

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

int main() {
    FILE * fp;
    char * filename = "Carss.txt";
    char lines[100000], blank[100000];
    char * token, * del = " ";
    int i = 0, c, d = 0, j;
    char * make[10000];
    char * year[10000];
    if ((fp = fopen(filename, "r")) == NULL) /* Opens the file */ {
        printf("can't open %s\n", filename);
        exit(1);
    }
    c = 0;
    while (fgets(lines, sizeof(lines), fp) != NULL) {
        token = strtok(lines, "\t");
        i = 1;
        while (i < 5) {
            token = strtok(NULL, "\t");
            if (i == 2) {
                make[c] = token;
            }
            if (i == 4) {
                year[c] = token;
            }
            i++;
        }
        printf("%s\n", make[c]); /* OUTPUT OF THE ARRAY HERE IS WHAT I WANTED */
        c++;
    }
    c = 0;
    while (c < 3) {
        printf("%d\n", c);
        printf("%s\n", make[c]);
        c++;
    }
}

The Text File I tokenize: Carss.txt

1   02V288000   FORD    FOCUS   2001    02S41   ELECTRICAL SYSTEM:BATTERY:CABLES    FORD MOTOR COMPANY  19990719    20010531    V   291854  20030210    ODI Ford Motor Company  20021106    20021106            CERTAIN PASSENGER VEHICLES EQUIPPED WITH ZETEC ENGINES, LOOSE OR BROKEN ATTACHMENTS AND MISROUTED BATTERY CABLES COULD LEAD TO CABLE INSULATION DAMAGE. THIS, IN TURN, COULD CAUSE THE BATTERY CABLES TO SHORT RESULTING IN HEAT DAMAGE TO THE CABLES.  BESIDES HEAT DAMAGE, THE "CHECK ENGINE" LIGHT MAY ILLUMINATE, THE VEHICLE MAY FAIL TO START, OR SMOKE, MELTING, OR FIRE COULD ALSO OCCUR.   DEALERS WILL INSPECT THE BATTERY CABLES FOR THE CONDITION OF THE CABLE INSULATION AND PROPER TIGHTENING OF THE TERMINAL ENDS.  AS NECESSARY, CABLES WILL BE REROUTED, RETAINING CLIPS INSTALLED, AND DAMAGED BATTERY CABLES REPLACED.   OWNER NOTIFICATION BEGAN FEBRUARY 10, 2003.   OWNERS WHO DO NOT RECEIVE THE FREE REMEDY  WITHIN A REASONABLE TIME SHOULD CONTACT FORD AT 1-866-436-7332.    ALSO CONTACT THE NATIONAL HIGHWAY TRAFFIC SAFETY ADMINISTRATION'S AUTO SAFETY HOTLINE AT 1-888-DASH-2-DOT (1-888-327-4236). 000015339000215022000000202
2   02V288000   FORD    FOCUS   2000    02S41   ELECTRICAL SYSTEM:BATTERY:CABLES    FORD MOTOR COMPANY  19990719    20010531    V   291854  20030210    ODI Ford Motor Company  20021106    20021106            CERTAIN PASSENGER VEHICLES EQUIPPED WITH ZETEC ENGINES, LOOSE OR BROKEN ATTACHMENTS AND MISROUTED BATTERY CABLES COULD LEAD TO CABLE INSULATION DAMAGE. THIS, IN TURN, COULD CAUSE THE BATTERY CABLES TO SHORT RESULTING IN HEAT DAMAGE TO THE CABLES.  BESIDES HEAT DAMAGE, THE "CHECK ENGINE" LIGHT MAY ILLUMINATE, THE VEHICLE MAY FAIL TO START, OR SMOKE, MELTING, OR FIRE COULD ALSO OCCUR.   DEALERS WILL INSPECT THE BATTERY CABLES FOR THE CONDITION OF THE CABLE INSULATION AND PROPER TIGHTENING OF THE TERMINAL ENDS.  AS NECESSARY, CABLES WILL BE REROUTED, RETAINING CLIPS INSTALLED, AND DAMAGED BATTERY CABLES REPLACED.   OWNER NOTIFICATION BEGAN FEBRUARY 10, 2003.   OWNERS WHO DO NOT RECEIVE THE FREE REMEDY  WITHIN A REASONABLE TIME SHOULD CONTACT FORD AT 1-866-436-7332.    ALSO CONTACT THE NATIONAL HIGHWAY TRAFFIC SAFETY ADMINISTRATION'S AUTO SAFETY HOTLINE AT 1-888-DASH-2-DOT (1-888-327-4236). 000015339000215021000000202
3   02V236000   JAYCO   FT EAGLE 10 SG  2003        EQUIPMENT:OTHER:LABELS  JAYCO, INC. 20020730    20020813    V   86  20020923    MFR Jayco, Inc. 20020904    20020912            ON CERTAIN FOLDING TENT CAMPERS, THE FEDERAL CERTIFICATION (AND RVIA) LABELS HAVE THE INCORRECT GROSS VEHICLE WEIGHT RATING, TIRE SIZE, AND INFLATION PRESSURE LISTED.  IF THE TIRES WERE INFLATED TO 80 PSI, THEY COULD BLOW RESULTING IN A POSSIBLE CRASH.    OWNERS WILL BE MAILED CORRECT LABELS FOR INSTALLATION ON THEIR VEHICLES.   OWNER NOTIFICATION BEGAN SEPTEMBER 23, 2002.    OWNERS SHOULD CONTACT JAYCO AT 1-877-825-4782.   ALSO, CUSTOMERS CAN CONTACT THE NATIONAL HIGHWAY TRAFFIC SAFETY ADMINISTRATION'S AUTO SAFETY HOTLINE AT 1-888-DASH-2-DOT (1-888-327-4236).  000015210000106403000000349

Upvotes: 3

Views: 158

Answers (4)

AndersK
AndersK

Reputation: 36082

strtok operates on a static buffer

when you write

strtok(lines, "\t");

the lines buffer is copied to an internal static buffer - and strtok has only one static buffer - so every time your while loop iterates, new content is placed intothe static buffer invalidating any previous pointers i.e. make[] and year[].

in order to preserve the tokens you need to allocate separate memory for them which is not destroyed at the end of the loop. this can be done in several ways like using strdup on each string like mentioned by other posters although there would be a lot of bookkeeping then freeing the meory. you could also use another data structure, e.g. a linked list to store the values instead of having lots of pointers:

typedef struct record
{
  char make[32]; // lets say the max length of make is 32
  char year[10];  
  struct record* next;
} record;

whenever you read a new line from the file create a new record:

record* first = NULL;
record* last = NULL; 

while (fgets(lines, sizeof(lines), fp) != NULL) 
{
  // first create the record
  record* newRecord = malloc(sizeof(record));
  newRecord->next = NULL;

  // parse the line
  int column = 0;
  for (char* token = strtok(lines,"\t"); token != NULL; token = strtok(NULL, "\t"), ++column)
  {
    switch(column)
    {
    case 2: 
      strncpy(newRecord->make, token, sizeof(newRecord.make));
      break;
    case 4:
      strncpy(newRecord->year, token, sizeof(newRecord.year));
      break;
    default:  // ignore
      break;
    }
  }
  // put in the list
  if (first == NULL)
  {
    first = last = newRecord;
  }
  else
  {
    last->next = newRecord;
    last = newRecord;
  }
}

now you have the parts you are interested in a list. you can print out the list with

for (record* rec = first; rec != NULL; rec = rec->next)
{
  printf( "%s, %s\n", rec->make, rec->year );
}

// disclaimer, haven't compiled this.

Upvotes: 0

Leonardo Herrera
Leonardo Herrera

Reputation: 8406

You are using the wrong data structure to store your results.

The make array is an array of pointers. This means that all you store there is a memory address, in your case, the memory address of the pointer being used during the strtok operation. This is not what you want, because you don't know what is going to happen with that memory afterwards; in this case, it appears that during each successive call to the function it gets overwritten.

You need to either use a multidimensional array or to use malloc() or strdup() to create a copy of the char * value as returned by strtok().

Upvotes: 0

ams
ams

Reputation: 25579

When you do this:

token = strtok(NULL, "\t");
if(i==2)
{
    make[c] = token;
}

You do not take a copy of the string, only a pointer reference to it. Then the next time you call fgets the string is overwritten, and all the references point to the new string. So, when you get to the end of the program all your pointers point to the last thing that was written to that memory.

Try this:

make[c] = strdup(token);

That will allocate new memory, duplicate the string, and return the new pointer. The copy will not get overwritten. strdup calls malloc, internally, so you should free the memory when you are done with it.

Upvotes: 4

Useless
Useless

Reputation: 67743

Inside the loop, make[c] = token records an address inside char lines[]. Since that buffer is overwritten on the next iteration, you need to copy your tokens somewhere else to keep them safe.

Just use make[c] = strdup(token) (and the same for year) to do this - but remember in any real program, you should now free all your make and year strings when you're done with them.

Upvotes: 1

Related Questions