0
#include<stdio.h>
#include<stdlib.h>
#include<string.h>
void main(){
        FILE *fp;
        fp=fopen("test","r");

        if(fp=NULL){
                printf("Error in opening file\n");
                exit(0);
        }
        else{
                char str[15],arr[200],temp,*add;
                add=arr;
                int i;

                while(!feof(fp)){
                        fscanf(fp,"%s",str);
                        for(i=0;i<(strlen(str)/2)+1;i++){
                                temp=str[i];
                                str[i]=str[strlen(str)-(i+1)];
                                str[strlen(str)-(i+1)]=temp;
                        };
                        strcpy(add,str);
                        add+=(strlen(add)+1);
                        *(add-1)=' ';
                };
                *(arr+strlen(arr))='\0';
                printf("%s \n",arr);
        };
        fclose(fp);
}

This code is written to read text from a file named test that contains only words and spaces and multiple lines.Whenever I try to execute it shows segmentation fault.

Achal
  • 11,821
  • 2
  • 15
  • 37
Ryuzaki
  • 17
  • 5
  • 3
    Have you tried using a debugger to see where the code breaks? – Immac Feb 05 '18 at 19:12
  • 3
    Formatting and indentation is not important for the compiler, but it is for people reading your code. Please try to format and indent it nicely so it's easier for us to read and understand. – Some programmer dude Feb 05 '18 at 19:13
  • 5
    I also recommend you read [Why is “while ( !feof (file) )” always wrong?](https://stackoverflow.com/questions/5431941/why-is-while-feof-file-always-wrong) – Some programmer dude Feb 05 '18 at 19:14
  • 2
    Lastly a hint: If the string in `arr` is not terminated when you do `*(arr+strlen(arr))='\0'`, what do you think the call to `strlen(arr)` will do? – Some programmer dude Feb 05 '18 at 19:15
  • 1
    fp will always be null because of `fp = NULL`, It should be `if (fp == NULL)`. From there on fp is stdin. –  Feb 05 '18 at 19:20
  • there are only two valid signatures for the `main()` function. (regardless of what visual studio might allow) they both have a return type of `int`, I.E. `void` is an error – user3629249 Feb 05 '18 at 21:31
  • regarding: ` printf("Error in opening file\n");` Error messages should be output to `stderr`, not `stdout`. When the error is from a system function (other than any of the `scanf()` family) then should call `perror()` as that outputs the enclosed text and the text reason the system thinks the error occurred to `stderr`. – user3629249 Feb 05 '18 at 21:33
  • regarding: `fscanf(fp,"%s",str);` When calling any of the `scanf()` family of functions, always check the returned value (not the parameter values) to assure the operation was successful. When using the input format specifier '%s' or '%[...]', always include a MAX CHARACTERS modifier that is one less than the length of the input buffer. because a) those input format specifiers always append a NUL byte to the input and b) to avoid any possibility of a buffer overflow, which would be undefined behavior and can lead to a seg fault event – user3629249 Feb 05 '18 at 21:37
  • for ease of readability and understanding: 1) separate code blocks ( `for` `if` `else` `while` `do...while` `switch` `case` `default` ) via a single blank line. 2) insert appropriate space inside parens, after commas, after semicolons, around C operators 3) use meaningful variable (and parameter) names. Names should indicate `content` or `usage` (or better, both) – user3629249 Feb 05 '18 at 21:41
  • regarding: `while(!feof(fp)){ fscanf(fp,"%s",str);` the function: `feof()` does not do what the code is expecting it to do. Suggest replacing these two lines with; `while( 1 == fscanf( fp, "%14s", str ) )` – user3629249 Feb 05 '18 at 21:43
  • My Turbo C compiler from 1989 says "possibly incorrect assignment" when I compile this. Either you are using a compiler which is worse than Turbo C from 1989, you have have no warnings enabled. – Lundin Feb 28 '18 at 10:48
  • @user3629249 No that advise is complete nonsense. What you refer to is the "yoda conditions" and they went obsolete in the year 1989, when compilers started to warn for "possibly incorrect assignment". Please don't recommend people to pick up a confused coding style from the early 80s. Use proper compilers instead. – Lundin Feb 28 '18 at 10:52
  • @Lundin, Exactly which 'advice' are you referring to as being 'obsolete'? – user3629249 Feb 28 '18 at 23:40
  • @user3629249 Placing the constant first, `while(1 == fscanf(...)`. This is the "yoda conditions", which is a very obsolete trick from the 1980s, that should neither be used nor recommended today. – Lundin Mar 01 '18 at 07:27

1 Answers1

4

The line 

if(fp=NULL){

Should be

if(fp==NULL){

gcc with -Wall option should have warn you about this.

I advice you to write if (NULL == fp){... that way, if == becomes a =, the compiler won't emit a warning, but an error telling you that you cannot change the NULL value...

Mathieu
  • 8,840
  • 7
  • 32
  • 45
  • 1
    Or `if(!fp) {...}` ?? – machine_1 Feb 05 '18 at 19:20
  • 2
    Something nice to get in the habit of for developers in general is to put the constant first so that the compiler will generate an error, whether you are developing in C, C++ or Java. `if (null == fp)`. If you had written `if (null = fp)`, this would have generated a compiler error in any language. Just to re-iterate what purplepsycho said. – jiveturkey Feb 05 '18 at 19:29
  • @jiveturkey No that advise is complete nonsense. What you refer to is the "yoda conditions" and they went obsolete in the year 1989, when compilers started to warn for "possibly incorrect assignment". Please don't recommend people to pick up a confused coding style from the early 80s. Use proper compilers instead. – Lundin Feb 28 '18 at 10:49
  • 1
    @Lundin, if your speaking about placing the literal first in a comparison. *I* have often seen, even with modern (latest version) compilers that no warning message is output when the literal is on the right. There is nothing 'yoda' about 'defensive' coding practices. – user3629249 Feb 28 '18 at 23:44
  • @user3629249 The "yoda conditions" have been criticized since the 1980s since they obfuscate the code. Since the year 1989, they have little to do with defensive programming, since it's all about enabling compiler warnings. The gcc compiler warns for this even without `-Wall -Wextra`. So did Turbo C from 1989. There's no excuse for compilers not to warn for this in the year 2018. And it's also the programmer's responsibility to enable warnings and not use crap compilers, just as it is their responsibility not to use "clever" tricks that obfuscate the code. – Lundin Mar 01 '18 at 07:25