0
#include "stdafx.h"
#include "string.h"
#include <stdlib.h>

int _tmain(int argc, _TCHAR* argv[])
{
    FILE *fp1, *fp2;
    char ime[32], prezime[32]; char brojbodova[5];
    fp1 = fopen("ulaz.txt", "r");
    char linija[100];
    char linijacopy[100];
while(fgets(linija, sizeof(linija), fp1) != NULL)
    {
/* or this: while (!feof(fp1))
        {
            fgets(linija, sizeof(linija), (FILE*)fp1);
            rest of code*/

    strcpy(linijacopy, linija);
    strcpy(ime, strtok(linija , " "));
    strcpy(prezime, strtok(NULL, " "));
    strcpy(brojbodova, strtok(NULL, " "));
    int bbodova = atoi(brojbodova);
    if(bbodova <= 50)
    {
        printf("%s\n", linijacopy);

    }

   }
fclose(fp1);
    return 0;
}

When I build this solution I doesn't have any error, but when I run this program, after printing results I got following windows error:

Console application has stopped working.

Windows can check online for a solution to the problem.

And there are three options:

-Check online...

-Close the program

-Debug the program

Problem details:

Problem signature: Problem Event Name: APPCRASH
Application Name: ConsoleApplication8.exe 
Application Version: 0.0.0.0 
Application Timestamp: 5834b1c7 
Fault Module Name: MSVCR110D.dll 
Fault Module Version: 11.0.50727.1 
Fault Module Timestamp: 5011aa23 
Exception Code: c0000005 
Exception Offset: 0008f7b3 
OS Version: 6.1.7601.2.1.0.256.1 
Locale ID: 9242 
Additional Information 1: 0a9e 
Additional Information 2: 0a9e372d3b4ad19135b953a78882e789 
Additional Information 3: 0a9e 
Additional Information 4: 0a9e372d3b4ad19135b953a78882e789

This program was typed in VS 2012 and OS is Windows 7 32 Enterprise.

Jonathan Leffler
  • 730,956
  • 141
  • 904
  • 1,278
FrankCottrill
  • 1
  • Your program is very unsafe. As a rule -> DO NOT ASSUME THAT FUNCTIONS RETURN WHAT YOU EXPECT. Check the return value instead of assuming. For example `strtok()` returns `NULL` very easily, and you passed it's return value as the second parameter to `strcpy()`. – Iharob Al Asimi Nov 22 '16 at 21:34
  • You should check the return value from every `strtok` before passing it to another function. It can be `NULL`. Never assume that any data input is what you expect it to be. – Weather Vane Nov 22 '16 at 21:34
  • for ease of readability and understanding: 1) follow the axiom: *only one statement per line and (at most) one variable declaration per statement.* 2) consistently indent the code: indent after every opening brace '{'. unindent before every closing brace '}'. 3) separate code blocks (for, if, else, while, do...while, switch, case, default) via a single blank line. – user3629249 Nov 22 '16 at 21:42
  • Note: never use `feof()` for loop control, it does not do what would be assumed that it does. – user3629249 Nov 22 '16 at 21:43
  • the header file: `string.h` is a system header file, so this statement: `#include "string.h"` should be: `#include ` The header file: 'stdafx.h` is compiled headers and is not portable. In the current scenario, suggest: `#include ` – user3629249 Nov 22 '16 at 21:46
  • the function name: `_tmain()` and the type: `_TCHAR` are only available under MSN` (visual studio). Strongly suggest, for portability and other reasons to use, respectably" `main()` and `char` – user3629249 Nov 22 '16 at 21:49
  • the posted code contains several 'magic' numbers: 5, 32, 50, 100. 'magic' numbers are numbers with no basis. Strongly suggest using a `enum` statement or `#define` statements to give those 'magic' numbers meaningful names and using those meaningful names throughout the code. – user3629249 Nov 22 '16 at 21:51
  • 1
    See [`while (!feof(file))` is always wrong](http://stackoverflow.com/questions/5431941/while-feof-file-is-always-wrong) for a deeper discussion of why using `feof()` as a loop control is invariably deeply suspect and almost always wrong. – Jonathan Leffler Nov 22 '16 at 21:51
  • 1
    You've not shown the example input data, nor the expected or actual results. You've got a lot of useful information in the comments about the issues with your code, but it is hard to give a concrete diagnosis of what is going wrong when you don't show the data. You could be running into buffer overflows; you could be accessing null pointers; there could be other problems not yet realized. Please read about how to create an MCVE ([MCVE]); that involves more than just the code. – Jonathan Leffler Nov 22 '16 at 21:54
  • strongly suggest re-declaring the variables: `ime`, `prezime`, and `brojbodova` as `char *` rather than arrays, then use `strdup()` to set each of those pointers from the pointer(s) returned from `strtok()`. (naturally, the code should pass each of those three pointers to `free()` before exiting. It is highly likely that the current calls to `strcpy()` are either copying from address 0 (NULL) or overrunning the arrays. Either case is undefined behavior and can lead to a (as you saw) seg fault event. – user3629249 Nov 22 '16 at 21:54
  • when compiling, always enable all the warnings, then fix those warnings. The posted code causes the compiler to output three(3) warnings. `warning: unused variable 'fp2'` `warning: unused parameter 'argc'` `warning: unused parameter 'argv'` The 'fp2' problem can be eliminated by eliminating that variable. The 'argc' and 'argv' problems can be eliminated by using the other valid signature for the `main()` function. I.E. `int main( void )` – user3629249 Nov 22 '16 at 22:02

2 Answers2

0

Your problem is assuming every function returns valid pointers, that's wrong

  1. You MUST check the return value of fopen(), otherwise you will dereference a NULL pointer.

  2. You MUST check the return value of strtok(), the same problem &rightarrow; NULL pointer dereference.

Iharob Al Asimi
  • 52,653
  • 6
  • 59
  • 97
0

the following code addresses all the concerns itemized in the question comments and greatly simplifies the code by elimination of unneeded variables, code logic, etc.

//#include "stdafx.h"
#include <stdio.h>
#include <string.h>
#include <stdlib.h>

#define MAX_LINE_LEN (100)

int main( void )
{
    FILE *fp1 = NULL;
    char *brojbodova;

    if( NULL == (fp1 = fopen("ulaz.txt", "r") ) )
    { // fopen failed
        perror( "fopen for ulaz.txt failed" );
        exit( EXIT_FAILURE );
    }

    // implied else, fopen successful

    char linija[ MAX_LINE_LEN ];
    char linijacopy[ MAX_LINE_LEN ];

    while(fgets(linija, sizeof(linija), fp1))
    {
        strcpy(linijacopy, linija);

        if( strtok(linija , " "))
        {
            if( strtok(NULL, " "))
            {
                if( NULL != (brojbodova = strtok(NULL, " ") ) )
                {
                    int bbodova = atoi(brojbodova);
                    if(bbodova <= 50)
                    {
                        printf("%s\n", linijacopy);
                    }
                }

                else
                { // invalid line format
                    printf( "invalid line format, exiting: %s\n", linijacopy );
                }
            }

            else
            { // invalid line format
                printf( "invalid line format, exiting: %s\n", linijacopy );
            }
        }

        else
        { // invalid line format
            printf( "invalid line format, exiting: %s\n", linijacopy );
        }
   } // end while

    fclose(fp1);
} // end function: main

the above code works correctly on my computer.

And since I have no file in the local directory named: ulaz.txt, it outputs the appropriate system message.

Output:

fopen for ulaz.txt failed: No such file or directory
user3629249
  • 16,402
  • 1
  • 16
  • 17
  • Suggest using `strtol()` rather than `atoi()` as `strtol()` performs error checking. Suggest reading the man page for `strtol()` for the details – user3629249 Nov 22 '16 at 22:34