Segmentation Fault

[Glen]

i get segmentation fault when i execute the following code(the
platform is gnu/linux)
#include<stdio.h>

int main()
{
char fn1[50],fn2[50],tn[50];
printf("\nEnter the first file name :");
scanf("%s",fn1);
printf("\nEnter the second file name :");
scanf("%s",fn2);
printf("\nEnter the target file name :");
scanf("%s"tn);
jfilejoin(&fn1,&tn);
jfilejoin(fn2,tn);
return 0;
}

int jfilejoin(char *fname,char *tfname)
{
FILE *in,*out;
int i,ch;
if((out=fopen(tfname,"a"))==NULL)
out=fopen(fname,"w");

in=fopen(fname,"r");

while(!feof(in))
{
ch=fgetc(in);
if(!feof(in))
fputc(ch,out);
}
fclose(in);
fclose(out);
return 0;
}

kindly help.this pgm is supposed to join two files to the target
filename.this is a module of a multithreaded download manager;-)
thanx

 

[Malcolm]

#include<stdio.h>

int main()
{
char fn1[50],fn2[50],tn[50];
printf("\nEnter the first file name :");
scanf("%s",fn1);
printf("\nEnter the second file name :");
scanf("%s",fn2);
printf("\nEnter the target file name :");
scanf("%s"tn);

/*
missing comma. Did you retype instead of cut and paste?
scanf() may be causing your problems here, since if the filenames are more
than 49 characters you will overwrite memory.
*/

jfilejoin(&fn1,&tn);
jfilejoin(fn2,tn);

Why are you passing &fn1 in one call and fn2 in the other? Both are valid,
though the second form is the convention.

return 0;
}

int jfilejoin(char *fname,char *tfname)
{
FILE *in,*out;
int i,ch;
if((out=fopen(tfname,"a"))==NULL)
out=fopen(fname,"w");

in=fopen(fname,"r");

You need to check in and out for NULL. This could well be your problem -
for some reasons the filenames you are passing can't be opened, or don't
have permissions.

while(!feof(in))

{
ch=fgetc(in);
if(!feof(in))
fputc(ch,out);
}
fclose(in);
fclose(out);

Check fclose if the data is valuable. fclose can also fail, leaving you with
a half-written file.

 

[Barry Schwarz]

i get segmentation fault when i execute the following code(the
platform is gnu/linux)

Have you stepped through the code with your debugger? On which
statement does it fail?

#include<stdio.h>

int main()
{
char fn1[50],fn2[50],tn[50];
printf("\nEnter the first file name :");
scanf("%s",fn1);

How many characters are the file path/names you use in your test?
Anything over 49 explains everything.

printf("\nEnter the second file name :");
scanf("%s",fn2);
printf("\nEnter the target file name :");
scanf("%s"tn);

Typo? Where is the real source?

jfilejoin(&fn1,&tn);

Since there is no prototype in scope for jfilejoin, your compiler
cannot warn you that your arguments have the wrong type. Both
arguments have type char (*)[50] but your function expects both to be
char*.

While technically this invokes undefined behavior, it is probably not
the cause of your problem. On most systems, both types of pointers
have the same representation and are passed the same way.

jfilejoin(fn2,tn);
return 0;
}

int jfilejoin(char *fname,char *tfname)
{
FILE *in,*out;
int i,ch;
if((out=fopen(tfname,"a"))==NULL)
out=fopen(fname,"w");

This is mind boggling. What was your intent? Surely the second fopen
uses tfname also. If the first fails, what makes you think the second
can possibly succeed?

in=fopen(fname,"r");

All I/O operations should always be checked for success.

while(!feof(in))
{
ch=fgetc(in);
if(!feof(in))
fputc(ch,out);
}
fclose(in);
fclose(out);
return 0;
}

kindly help.this pgm is supposed to join two files to the target
filename.this is a module of a multithreaded download manager;-)
thanx

<<Remove the del for email>>

 

[Christopher Benson-Manica]

char fn1[50],fn2[50],tn[50];
scanf("%s",fn1);
scanf("%s",fn2);
scanf("%s"tn);

As others stated, your scanf's here are asking for trouble - they're
about as good an idea as gets is. Consider using fgets instead
(assuming your input is delimited by newlines):

fgets( fn1, sizeof fn1, stdin );
fgets( fn2, sizeof fn2, stdin );
fgets( tn, sizeof tn, stdin );

If you must use scanf (for masochistic reasons or other), specify the
maximum field width like so:

scanf( "%50s", fn1 );
scanf( "%50s", fn2 );
scanf( "%50s", tn );

Notice that if you change the declarations of fn1, fn2, or tn, you
must also change the scanf calls. Note also that you needn't do so
with the fgets calls, which is one of many reasons to prefer fgets
when possible.

 

[Dan Pop]

char fn1[50],fn2[50],tn[50];
scanf("%s",fn1);
scanf("%s",fn2);
scanf("%s"tn);

As others stated, your scanf's here are asking for trouble - they're
about as good an idea as gets is. Consider using fgets instead
(assuming your input is delimited by newlines):

fgets( fn1, sizeof fn1, stdin );
fgets( fn2, sizeof fn2, stdin );
fgets( tn, sizeof tn, stdin );

If you must use scanf (for masochistic reasons or other), specify the

^^^^^
Like because it is the best tool for the job, if used correctly.

maximum field width like so:

scanf( "%50s", fn1 );
scanf( "%50s", fn2 );
scanf( "%50s", tn );

Notice that if you change the declarations of fn1, fn2, or tn, you
must also change the scanf calls. Note also that you needn't do so
with the fgets calls, which is one of many reasons to prefer fgets
when possible.

fgets has its own set of problems, much worse than those of scanf in
competent hands. Your own usage of fgets is incorrect, BTW, which
proves my point.

Dan

 

[Christopher Benson-Manica]

fgets has its own set of problems, much worse than those of scanf in
competent hands. Your own usage of fgets is incorrect, BTW, which
proves my point.

Um, quite so. I would still maintain, however, that more questions
have been posted here that are the result of using scanf incorrectly
than of using fgets incorrectly. Why do you say that fgets is much
worse than scanf?

 

[Rob Thorpe]

Um, quite so. I would still maintain, however, that more questions
have been posted here that are the result of using scanf incorrectly
than of using fgets incorrectly. Why do you say that fgets is much
worse than scanf?

This issue is blurred because scanf is much more generally useful. So
there are more mistakes made using it.

 

[Dan Pop]

Um, quite so. I would still maintain, however, that more questions
have been posted here that are the result of using scanf incorrectly
than of using fgets incorrectly.

If you compare the specifications of the two functions, this is to be
expected. A Leatherman tool or a Swiss army knife can be misused in a
lot more ways than a plain knife.

Why do you say that fgets is much worse than scanf?

Try to use it in a *bullet-proof* manner in order to read at most
100 characters and dump the rest of the characters, if the input
line is longer. Try to do the same thing using getc instead and see
if fgets did buy you anything. Finally, try to solve the same
problem using scanf.

Dan

 

[Glen]

thank u friends for ur replies...
well the main problem was with opening the file..ie the input file..
well it became alright wehn i gave the absolute path as the input..
well i retyped some part of the code while posting...so soem errors
crept in sorry about that..
well the working code is this..
#include<stdio.h>

int main()
{
char fn1[50],fn2[50],tn[50];
scanf("%s %s %s",fn1,fn2,tn);
jfilejoin(fn1,tn);
jfilejoin(fn2,tn);
return 0;
}

int jfilejoin(char *fname,char *tfname)
{
FILE *in,*out;
int i,ch;
if((in=fopen(fname,"r"))==NULL)
{
printf("\nfile not opened %s %s\n",fname,tfname);
return;
}
if((out=fopen(tfname,"r"))==NULL)
{
out=fopen(tfname,"w");
printf("\nOpened file for writing\n");
fclose(out);
}
out=fopen(tfname,"a");
while(!feof(in))
{
ch=fgetc(in);
if(!feof(in))
fputc(ch,out);
}
fclose(in);
fclose(out);
return 0;
}

well about using fgets...i am giving it a thought ;-)
thanx
glen

 

[Barry Schwarz]

thank u friends for ur replies...
well the main problem was with opening the file..ie the input file..
well it became alright wehn i gave the absolute path as the input..
well i retyped some part of the code while posting...so soem errors
crept in sorry about that..
well the working code is this..
#include<stdio.h>

int main()
{
char fn1[50],fn2[50],tn[50];
scanf("%s %s %s",fn1,fn2,tn);
jfilejoin(fn1,tn);
jfilejoin(fn2,tn);
return 0;
}

int jfilejoin(char *fname,char *tfname)
{
FILE *in,*out;
int i,ch;
if((in=fopen(fname,"r"))==NULL)
{
printf("\nfile not opened %s %s\n",fname,tfname);
return;
}
if((out=fopen(tfname,"r"))==NULL)
{
out=fopen(tfname,"w");

If the fopen for read failed because the file exists but you do not
have access, this will probably fail also.

printf("\nOpened file for writing\n");
fclose(out);
}
out=fopen(tfname,"a");

And if those failed, this one will also.

If the fopen in the if succeeded, you now have opened tfname twice
using the same FILE*. Usually not a good idea.

while(!feof(in))
{
ch=fgetc(in);
if(!feof(in))
fputc(ch,out);
}
fclose(in);
fclose(out);
return 0;
}

well about using fgets...i am giving it a thought ;-)
thanx
glen

<<Remove the del for email>>

 

[Dave Thompson]

char fn1[50],fn2[50],tn[50];
scanf("%s",fn1);
scanf("%s",fn2);
scanf("%s"tn);

As others stated, your scanf's here are asking for trouble - they're
about as good an idea as gets is. Consider using fgets instead <snip>
If you must use scanf (for masochistic reasons or other), specify the
maximum field width like so:

scanf( "%50s", fn1 );
scanf( "%50s", fn2 );
scanf( "%50s", tn );

%49s; or change the defined bound to 51, perhaps as MACRO+1 where you
can concatenate stringized simple number MACRO into the *scanf format
string. The count to fgets includes room for the terminating null, the
field width to *scanf %s and %[ does not. (And %c doesn't terminate.)

I can't believe Dan Pop didn't already correct this

Notice that if you change the declarations of fn1, fn2, or tn, you
must also change the scanf calls. Note also that you needn't do so
with the fgets calls, which is one of many reasons to prefer fgets
when possible.

Unless you can use the macro trick above; or you generate the *scanf
format at runtime, but in most cases this is even yuckier:
char fmt [ 3*sizeof size_t + 1 /* (more than) enough for number */
+ 2 /* percent, ess */ + 1 /* null */ ];
sprintf (fmt, "%%%ld%s", (long)(sizeof buf -1));
if( scanf (fmt, buf) != 1 ) /* EOF or error */ ...

Another, unrelated, advantage of fgets is that interactive input
doesn't skip leading newlines and thus seemingly fail to respond; and
for noninteractive input you can accurately count and report the line
number where incorrect or unexpected input was encountered.

- David.Thompson1 at worldnet.att.net

 

[Christopher Benson-Manica]

I can't believe Dan Pop didn't already correct this

Heh, it isn't often that you get the chance to pounce on people, eh?
Thanks for pointing it out.

Another, unrelated, advantage of fgets is that interactive input
doesn't skip leading newlines and thus seemingly fail to respond; and
for noninteractive input you can accurately count and report the line
number where incorrect or unexpected input was encountered.

I'm of the impression that most of the well-learned sages of clc favor
fgets to scanf, so I'm not sure how I feel about Dan's quixotic quest
to promote scanf

 

[Joe Wright]

Christopher Benson-Manica wrote:

[ snip ]

I'm of the impression that most of the well-learned sages of clc favor
fgets to scanf, so I'm not sure how I feel about Dan's quixotic quest
to promote scanf

How you feel about it? Or do you mean what you think about it?

My quess is that Dan has taken the time to learn the *scanf() stuff
and correctly senses that most of us haven't. fgets() is simpler and
allows us to avoid even looking up *scanf(). Dan thinks that's lazy
of us. And he's right.

 

[Christopher Benson-Manica]

How you feel about it? Or do you mean what you think about it?

I think the two expressions are more or less equivalent in context,
but yes, what I think about it (not to suggest that my opinion is
exceptionally important to Dan or anyone else here).

My quess is that Dan has taken the time to learn the *scanf() stuff
and correctly senses that most of us haven't. fgets() is simpler and
allows us to avoid even looking up *scanf(). Dan thinks that's lazy
of us. And he's right.

Perhaps, but many people much more experienced than myself seem to
prefer it...

 

[Dan Pop]

Christopher Benson-Manica wrote:

[ snip ]

I'm of the impression that most of the well-learned sages of clc favor
fgets to scanf, so I'm not sure how I feel about Dan's quixotic quest
to promote scanf

Have you solved the exercise I gave to you? All those who did have seen
the light ;-)

How you feel about it? Or do you mean what you think about it?

My quess is that Dan has taken the time to learn the *scanf() stuff
and correctly senses that most of us haven't. fgets() is simpler and
allows us to avoid even looking up *scanf(). Dan thinks that's lazy
of us. And he's right.

fgets is only *apparently* simpler to use than *any* of the alternatives.

Once you try to use it in a bullet-proof fashion you discover that it's
horribly designed and the solution is simpler if you don't use it at all.
If you push your research further, you cannot avoid the conclusion that
scanf provides the simplest solution (two lines of code).

Dan