f



what is wrong with this code.

Hi,

I am looking at C programming, after doing a basic course a few years ago
and doing none since.  I have been tinkering abut with some very basic
file stuff, but I get a segmentation fault when the code hits the fgets
function. The code is as follows:

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

int main(int argc, char **argv)
{
  char path[]="/home/ccx264/data";
  char input[100];
  FILE *fp;
  char *str;
  printf("%s",path);


  if(fopen(path,"r") == NULL){
    fprintf(stderr, "Cannot open file.\n");
    exit(EXIT_FAILURE);
  }
  if(*str = fgets(input, 100, fp) == NULL){
    fprintf(stderr, "Cannot read string.\n");
    fclose(fp);
    exit(EXIT_FAILURE);
    }
  printf("The file read: %s\n",str);
  fclose(fp);
  return EXIT_SUCCESS;
}

What is wrong with this code...please be gentle ;-)



-- 
Gareth Ansell
Infrastructure
Computing Services
Coventry University
0
Gareth
6/16/2004 2:06:56 PM
comp.std.c 1570 articles. 0 followers. Post Follow

4 Replies
210 Views

Similar Articles

[PageSpeed] 36

In <pan.2004.06.16.14.06.55.743704@no-spam-please.coventry.ac.uk> Gareth Ansell <g.ansell@no-spam-please.coventry.ac.uk> writes:

>I am looking at C programming, after doing a basic course a few years ago
>and doing none since.  I have been tinkering abut with some very basic
>file stuff, but I get a segmentation fault when the code hits the fgets
>function. The code is as follows:

This newsgroup is for discussing the C standard, not for helping people
learning C.  Use comp.lang.c next time.

>#include <stdio.h>
>#include <stdlib.h>
>
>int main(int argc, char **argv)
>{
>  char path[]="/home/ccx264/data";
>  char input[100];
>  FILE *fp;
>  char *str;
>  printf("%s",path);

Where is the newline character ending the line generated by this printf
call?

>  if(fopen(path,"r") == NULL){

And if fopen() succeeds, where are you storing its return value?

>    fprintf(stderr, "Cannot open file.\n");
>    exit(EXIT_FAILURE);
>  }
>  if(*str = fgets(input, 100, fp) == NULL){

This is severely broken:

1. fp is still uninitialised, so you cannot pass it to fgets.

2. The == operator has higher precedence than the = operator, so the
   controlling expression of the if statement is parsed as:

     *str = (fgets(input, 100, fp) == NULL)

   which is clearly not what you want.

3. The return type of fgets is char pointer, while the type of *str is
   char.  If it weren't for the mistake mentioned above, the compiler
   would have complained about incompatible types in assignment.

>    fprintf(stderr, "Cannot read string.\n");
>    fclose(fp);
>    exit(EXIT_FAILURE);
>    }
>  printf("The file read: %s\n",str);
>  fclose(fp);
>  return EXIT_SUCCESS;
>}

Fixing the bugs already described will make the rest of the program
work, too.

>What is wrong with this code...please be gentle ;-)

You're using expressions too complex for your current level of
understanding.  While fixing the bugs, break your expressions into
simpler ones.  Once you get it working, you can also try experimenting
with the more complex version you tried in the first place.

As a stylistic comment, try using more white space in your code.

Dan
-- 
Dan Pop
DESY Zeuthen, RZ group
Email: Dan.Pop@ifh.de
0
Dan
6/16/2004 3:11:04 PM
Gareth Ansell <g.ansell@no-spam-please.coventry.ac.uk> wrote:

> I have been tinkering abut with some very basic
> file stuff, but I get a segmentation fault when the code hits the fgets
> function.

This really belongs in comp.lang.c, not comp.std.c. I've set follow-ups
accordingly.

>   char path[]="/home/ccx264/data";
>   char input[100];
>   FILE *fp;
>   char *str;
>   printf("%s",path);
> 


>   if(*str = fgets(input, 100, fp) == NULL)

First, what does fgets() return? And what is the type of str? What the
type of *str?
Second, you dereference str without having pointed it at memory you own.

There's usually no reason to assign the return value of fgets() to
anything, btw; if it doesn't return a null pointer, it always returns
the destination pointer you passed it yourself, in this case input.

Richard
0
rlb (4118)
6/16/2004 3:27:38 PM
Firstly, apologies for posting off topic, I hadn't realised until after I
posted that I had sent to comp.std.c and not comp.lang.c, as a subscribe to
both.

Secondly thanks for the help,  I totally missed that I hadn't initialised
the file pointer with the call to fopen.

Gareth Ansell


0
Gareth
6/17/2004 1:40:24 PM
Groovy hepcat Richard Bos was jivin' on Wed, 16 Jun 2004 15:27:38 GMT
in comp.lang.c.
Re: what is wrong with this code.'s a cool scene! Dig it!

>Gareth Ansell <g.ansell@no-spam-please.coventry.ac.uk> wrote:
>
>> I have been tinkering abut with some very basic
>> file stuff, but I get a segmentation fault when the code hits the fgets
>> function.
>
>This really belongs in comp.lang.c, not comp.std.c. I've set follow-ups
>accordingly.
>
>>   char path[]="/home/ccx264/data";
>>   char input[100];
>>   FILE *fp;
>>   char *str;
>>   printf("%s",path);
>>   if(*str = fgets(input, 100, fp) == NULL)
>
>First, what does fgets() return? And what is the type of str? What the
>type of *str?
>Second, you dereference str without having pointed it at memory you own.
>
>There's usually no reason to assign the return value of fgets() to
>anything, btw; if it doesn't return a null pointer, it always returns
>the destination pointer you passed it yourself, in this case input.

  Also, comparison has higher precedence than assignment, so the last
line of code shown above parses as this:

if(*str = (fgets(input, 100, fp) == NULL))

Now, clearly this isn't right. I think what's intended is that str is
assigned the return value of fgets() and then compared to NULL:

if(NULL != (str = fgets(input, 100, fp)))

-- 

Dig the even newer still, yet more improved, sig!

http://alphalink.com.au/~phaywood/
"Ain't I'm a dog?" - Ronny Self, Ain't I'm a Dog, written by G. Sherry & W. Walker.
I know it's not "technically correct" English; but since when was rock & roll "technically correct"?
0
phaywood (258)
6/19/2004 1:07:13 AM
Reply: