using strcpy in a while loop, C

Question

I am new in programming and I am developing a C application that gets data from a file. I open and analyze every line of this external file. Inside the loop I use a parser function that I previously created in order to get the desired data and parse it.

This is working good but now I need to save every parsed line (every message) in another array named buffer. Every message is limited to 200 chars and buffer to 1000 chars, so in buffer array could be stored exactly 5 messages. My external file have more than 5 messages so when the 6th message arrive, the 1st message should be remove from buffer. I am getting problems here because I do not know how to store values in buffer without overriding it.

This is the main function:

void main(void) {

        FILE *fp = fopen ("file","r"); //open file
    char line[200]; 
    char parsed_line[200];
    char *message;
    char buffer [1000]; 

    if (fp==NULL) printf ("the file open was not succeed");
    else{
        printf ("Device open!");

        while(fscanf (fp, "%s", line) > 0 ){

                if(strstr(line, "$GPRMC")){

                message = parser(line, parsed_line); //gets a line and parse it
                strcpy(buffer, message);
            }
        }
    }
        return;
    }

I would be grateful if somebody could help me.

It would be better if you user memcpy() it takes start point and length . in this way you can manage your memory very easily — sam, Feb 22 '16 at 11:33
Why is everyone suggesting copying the messages? What is wrong with an array of 5 pointers? The message is returned as a pointer, presumably to a struct/array allocated with malloc, so why not just rotate the pointers? Why bulk-copy the data over and over? Is this a mistagged C++ question? :) — Martin James, Feb 22 '16 at 11:38
@AshishAhuja This is C, so `void main` is fine and standard in many cases. [Read this](http://stackoverflow.com/a/31263079/584518). — Lundin, Feb 22 '16 at 12:01
@picador You simply need to implement a ring buffer ADT which can accept any data type. — Lundin, Feb 22 '16 at 12:03
The `void main()` will cause the compiler to output a warning message in almost every compiler. The only time a `void main()` is acceptable is when working on a 'bare metal' (no OS) board (and sadly with visual studio). In any scenario with an OS, the OS is expecting a integer return value (that the shell can use for various purposes) — user3629249, Feb 22 '16 at 23:30

Umamahesh P · Accepted Answer · 2016-02-22T12:38:03.653

It is easy to use 2D char array as suggested in the comment and the loop can be changed as shown below.

char buffer[5][201];

msgcount = 0;
while(...)
{
   ...
   strcpy(buffer[msgcount], message);
   if( (++msgcount) >= 4 )
       msgcount = 0;
  ...
}

OR

while(...)
{
    ...
    parser(line, buffer[msgcount]); 
    if( (++msgcount) >= 4 )
    msgcount = 0;
}

The disadvantage here is size of the array is fixed. If you want to change the number of strings in future, then you can define array of char pointers with fixed size string. If you are not sure about size of the string and buffer size then you can use char** data type and then allocate memory.

Cantfindname · Answer 2 · 2016-02-22T11:46:16.177

The problem in your code is with strcpy. The way it is written, each message will write in the beginning of the buffer, overwriting previous messages. Also strcpy will copy a null-terminated string, which means that you will need to somehow keep each message's size in order to read them from the buffer. Instead, you can use memcpy, with which you can copy exactly 200 chars each time. So, if your messages are null-terminated, you know exactly where they start and finish inside the buffer. (This also means that if your messages are 200 chars not-including the \0, the buffer should be 1004 chars long)

void main(void) {

        FILE *fp = fopen ("file","r"); //open file
    char line[200]; 
    char parsed_line[200];
    char *message;
    char buffer [1000]; 

    if (fp==NULL) printf ("the file open was not succeed");
    else{
        printf ("Device open!");

        int message_number = 0;
        while(fscanf (fp, "%s", line) > 0 ){

                if(strstr(line, "$GPRMC")){

                message = parser(line, parsed_line); //gets a line and parse it
                memcpy(buffer + 200*message_number, message, 200);

                message_number = (message_number+1) % 5;

                if (message_number == 4){
                    //flush the buffer
                }
            }
        }
        fclose(fp);
    }
        return;
    }

I am using the maximum of 200 chars for each message in the buffer, so that you do not need to keep each one's size when you read from the buffer.

Explain what you have written. – Ashish Ahuja Feb 22 '16 at 11:40 — Ashish Ahuja, Feb 22 '16 at 11:40

using strcpy in a while loop, C

2 Answers2