program bug

  • Thread starter Bill Cunningham
  • Start date
B

Bill Cunningham

I have looked this program up and down and I don't see what's wrong with
it. But it always breaks and gives me an error "mode error" no matter which
mode binary or text I choose. This simple program is supposed to take as
argv[1] a "b" or "t" for binary or text. It's not taking anything.

#include <stdio.h>

int main(int argc, char **argv) {
char *b;
int a;
FILE *ifp,*ofp;
if (argc!=4) {
fprintf(stderr,"usage error\n");
return -1;
}
if (argv[1]=="b") {
b="rb";
}
if (argv[1]=="t") {
b="rt";
}
if (argv[1]!="t"||argv[1]!="b") {
fprintf(stderr,"mode error\n");
return -1;
}
if ((ifp=fopen(argv[2],b))==0) {
fprintf(stderr,"open error i\n");
return -1;
}
if ((ofp=fopen(argv[3],b))==0) {
fprintf(stderr,"open error o\n");
return -1;
}
while(a!=EOF)
a=fgetc(ifp);
fputc(a,ofp);
printf("done\n");
return 0;}

Is anyone good enough to glance at this and see what's wrong?

Bill
 
B

Bill Cunningham

Well I now see I left out fclose I will correct that but I don't think
that's my problem.

Bill
 
R

Robert Gamble

I have looked this program up and down and I don't see what's wrong with
it. But it always breaks and gives me an error "mode error" no matter which
mode binary or text I choose. This simple program is supposed to take as
argv[1] a "b" or "t" for binary or text. It's not taking anything.

#include <stdio.h>

int main(int argc, char **argv) {
char *b;
int a;
FILE *ifp,*ofp;
if (argc!=4) {
fprintf(stderr,"usage error\n");
return -1;
Click to expand...

Not a portable return value from main.
}
if (argv[1]=="b") {
Click to expand...

This isn't doing what you think it is. This is comparing the address
of first program argument to the address of the string literal "b"
which will always be different. Either use (strcmp(argv[1], "b") ==
0) or say:

if (argv[1][0] == 'b') {

which will compare the first letter of the first argument to the
character constant b.
b="rb";
}
if (argv[1]=="t") {
b="rt";
}
if (argv[1]!="t"||argv[1]!="b") {
fprintf(stderr,"mode error\n");
return -1;
}
if ((ifp=fopen(argv[2],b))==0) {
fprintf(stderr,"open error i\n");
return -1;
}
if ((ofp=fopen(argv[3],b))==0) {
fprintf(stderr,"open error o\n");
return -1;
}
while(a!=EOF)
Click to expand...

a is not initialized, referencing its value invokes undefined
behavior.
a=fgetc(ifp);
fputc(a,ofp);
Click to expand...

I think you want braces around these statements, yes?

Try (not tested):

while ( (a = fgetc(ifp)) != EOF && fputc(a, ofp) != EOF )
;
printf("done\n");
Click to expand...

puts("done"); would work equally well.
 
R

Robert Gamble

Well I now see I left out fclose I will correct that but I don't think
that's my problem.

Bill
Click to expand...

There were a number of problems with the program but as long as the
program was terminating normally the fclose() wasn't one of them, all
files are properly closed during normal program termination (although
it is usually a good idea to explicitly close files yourself).
 
B

Bill Cunningham

In the beginning a was declared. Is it not enough in this case to simply
declare an int? Or should I have done this int a=0; at the beginning of the
program?

Bill
 
B

Bill Cunningham

Not a portable return value from main.
}
if (argv[1]=="b") {
Click to expand...

This isn't doing what you think it is. This is comparing the address
of first program argument to the address of the string literal "b"
Click to expand...

Was declaring b as a pointer to char proper? Or should a simple b of
type char all that's needed?

which will always be different. Either use (strcmp(argv[1], "b") ==
0) or say:

if (argv[1][0] == 'b') {

which will compare the first letter of the first argument to the
character constant b.
Click to expand...

I do not understand the above but it looks like a good way to write
code. Why is a zero used as the second dimension of this array?

Bill
 
R

Robert Gamble

In the beginning a was declared. Is it not enough in this case to simply
declare an int? Or should I have done this int a=0; at the beginning of the
program?
Click to expand...

You need to store a value into a variable before you use the value of
that variable, either through initialization (implicit or explicit) or
assignment. If you don't do this then the variable may contain
garbage ("indeterminate value" in standards parlance), that garbage
may be a trap value which will invoke UB when read. Note that the
alternative I provided does not have this problem because "a" is
assigned before its value is used.
 
I

Ian Collins

Bill said:
I have looked this program up and down and I don't see what's wrong with
it. But it always breaks and gives me an error "mode error" no matter which
mode binary or text I choose. This simple program is supposed to take as
argv[1] a "b" or "t" for binary or text. It's not taking anything.

#include <stdio.h>

int main(int argc, char **argv) {
char *b;
int a;
FILE *ifp,*ofp;
if (argc!=4) {
fprintf(stderr,"usage error\n");
return -1;
}
if (argv[1]=="b") {
b="rb";
}
if (argv[1]=="t") {
b="rt";
}
if (argv[1]!="t"||argv[1]!="b") {
fprintf(stderr,"mode error\n");
return -1;
}
if ((ifp=fopen(argv[2],b))==0) {
fprintf(stderr,"open error i\n");
return -1;
}
if ((ofp=fopen(argv[3],b))==0) {
fprintf(stderr,"open error o\n");
return -1;
}
while(a!=EOF)
a=fgetc(ifp);
fputc(a,ofp);
printf("done\n");
return 0;}

Is anyone good enough to glance at this and see what's wrong?
Click to expand...
Someone stole the whitespace?

Why are you comparing characters with string literals? Surly your
compiler gave you some warnings?
 
R

Robert Gamble

Bill said:
I have looked this program up and down and I don't see what's wrong with
it. But it always breaks and gives me an error "mode error" no matter which
mode binary or text I choose. This simple program is supposed to take as
argv[1] a "b" or "t" for binary or text. It's not taking anything.
Click to expand...
#include <stdio.h>
Click to expand...
int main(int argc, char **argv) {
char *b;
int a;
FILE *ifp,*ofp;
if (argc!=4) {
fprintf(stderr,"usage error\n");
return -1;
}
if (argv[1]=="b") {
b="rb";
}
if (argv[1]=="t") {
b="rt";
}
if (argv[1]!="t"||argv[1]!="b") {
fprintf(stderr,"mode error\n");
return -1;
}
if ((ifp=fopen(argv[2],b))==0) {
fprintf(stderr,"open error i\n");
return -1;
}
if ((ofp=fopen(argv[3],b))==0) {
fprintf(stderr,"open error o\n");
return -1;
}
while(a!=EOF)
a=fgetc(ifp);
fputc(a,ofp);
printf("done\n");
return 0;}
Click to expand...
Is anyone good enough to glance at this and see what's wrong?
Click to expand...

Someone stole the whitespace?

Why are you comparing characters with string literals? Surly your
compiler gave you some warnings?
Click to expand...

My compiler didn't provide any warnings for this code, did yours?
 
B

Bill Cunningham

Ian Collins said:
Someone stole the whitespace?
Click to expand...

I was always told there were several choices in C indentation. These two
and maybe a third.

function { /*notice the one space after the first brace*/
} closing brace on a line by itself.
This is the style I use and another

function
{

body

}

With this style all braces are on lines by themselves.


Bill
 
I

Ian Collins

Robert said:
My compiler didn't provide any warnings for this code, did yours?
Click to expand...
Oops, my bad, I spotted the comparisons an didn't spot they were with
argv[n].
 
I

Ian Collins

Joe said:
Ian said:
Robert said:
Why are you comparing characters with string literals? Surly your
compiler gave you some warnings?
My compiler didn't provide any warnings for this code, did yours?
Click to expand...
Oops, my bad, I spotted the comparisons an didn't spot they were with
argv[n].
Click to expand...
So what?

if (argv[1] == "b")

is not an error the compiler need diagnose. Both arguments are of type
(char *) and the chances the two pointer values are equal is nil but the
expression is completely valid.
Click to expand...
I know, that's why I said I didn't spot the comparison was with argv[n].
If I had, I wouldn't have mentioned compiler warnings.
 
U

user923005

    I have looked this program up and down and I don't see what's wrong with
it. But it always breaks and gives me an error "mode error" no matter which
mode binary or text I choose. This simple program is supposed to take as
argv[1] a "b" or "t" for binary or text. It's not taking anything.

#include <stdio.h>

int main(int argc, char **argv) {
 char *b;
 int a;
 FILE *ifp,*ofp;
 if (argc!=4) {
 fprintf(stderr,"usage error\n");
 return -1;
 }
 if (argv[1]=="b") {
 b="rb";
 }
 if (argv[1]=="t") {
 b="rt";
 }
 if (argv[1]!="t"||argv[1]!="b") {
 fprintf(stderr,"mode error\n");
 return -1;
 }
 if ((ifp=fopen(argv[2],b))==0) {
 fprintf(stderr,"open error i\n");
 return -1;
 }
 if ((ofp=fopen(argv[3],b))==0) {
 fprintf(stderr,"open error o\n");
 return -1;
 }
 while(a!=EOF)
 a=fgetc(ifp);
 fputc(a,ofp);
 printf("done\n");
 return 0;}

    Is anyone good enough to glance at this and see what's wrong?
Click to expand...

Too many things for a glance. It requires a perusal.

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

int main(int argc, char **argv)
{
char *imode = 0;
char *omode = 0;
char option;
int ch = 0;

FILE *ifp,
*ofp;
if (argc != 4) {
fprintf(stderr, "usage error\n");
return EXIT_FAILURE;
}
option = *argv[1];
if (option == 'b') {
imode = "rb";
omode = "wb";
}
if (option == 't') {
/* This may or may not do something useful on your system */
/* The 't' in the mode is not portable, but is a fairly */
/* popular extension for text mode. */
imode = "rt";
omode = "wt";
}
if (option != 't' && option != 'b') {
fprintf(stderr, "mode error\n");
return EXIT_FAILURE;
}
if ((ifp = fopen(argv[2], imode)) == 0) {
fprintf(stderr, "open error i\n");
return EXIT_FAILURE;
}
if ((ofp = fopen(argv[3], omode)) == 0) {
fprintf(stderr, "open error o\n");
return EXIT_FAILURE;
}
/* I guess that you want to write what you read, hence the {} */
do {
ch = fgetc(ifp);
if (ch != EOF)
fputc(ch, ofp);
} while (ch != EOF);
fclose(ifp);
fclose(ofp);
printf("done\n");
return 0;
}
 
K

Keith Thompson

Bill Cunningham said:
I was always told there were several choices in C indentation.
Click to expand...
[...]

Yes, there are serveral choices. No indentation at all is not one of
them.

If you want help with your code, indent it. It doesn't matter much
which indentation style you choose, but pick *something* that reflects
the structure of your code. (To be clear, indentation refers to
whitespace at the beginning of each line.)
 
B

Bill Cunningham

user923005 said:
#include <stdio.h>

int main(int argc, char **argv) {
char *b;
int a;
FILE *ifp,*ofp;
if (argc!=4) {
fprintf(stderr,"usage error\n");
return -1;
}
if (argv[1]=="b") {
b="rb";
}
if (argv[1]=="t") {
b="rt";
}
if (argv[1]!="t"||argv[1]!="b") {
fprintf(stderr,"mode error\n");
return -1;
}
if ((ifp=fopen(argv[2],b))==0) {
fprintf(stderr,"open error i\n");
return -1;
}
if ((ofp=fopen(argv[3],b))==0) {
fprintf(stderr,"open error o\n");
return -1;
}
while(a!=EOF)
a=fgetc(ifp);
fputc(a,ofp);
printf("done\n");
return 0;}

Is anyone good enough to glance at this and see what's wrong?
Click to expand...

Too many things for a glance. It requires a perusal.

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

int main(int argc, char **argv)
{
char *imode = 0;
char *omode = 0;
char option;
int ch = 0;

FILE *ifp,
*ofp;
if (argc != 4) {
fprintf(stderr, "usage error\n");
return EXIT_FAILURE;
}
option = *argv[1];
if (option == 'b') {
imode = "rb";
omode = "wb";
}
if (option == 't') {
/* This may or may not do something useful on your system */
/* The 't' in the mode is not portable, but is a fairly */
/* popular extension for text mode. */
imode = "rt";
omode = "wt";
}
if (option != 't' && option != 'b') {
fprintf(stderr, "mode error\n");
return EXIT_FAILURE;
}
if ((ifp = fopen(argv[2], imode)) == 0) {
fprintf(stderr, "open error i\n");
return EXIT_FAILURE;
}
if ((ofp = fopen(argv[3], omode)) == 0) {
fprintf(stderr, "open error o\n");
return EXIT_FAILURE;
}
/* I guess that you want to write what you read, hence the {} */
do {
ch = fgetc(ifp);
if (ch != EOF)
fputc(ch, ofp);
} while (ch != EOF);
fclose(ifp);
fclose(ofp);
printf("done\n");
return 0;
}

I will definately save this code and study it. I really do no use do so
maybe this will open my eyes to it some.

Bill
 
V

vippstar

#include <stdio.h>
Click to expand...
int main(int argc, char **argv) {
char *b;
int a;
FILE *ifp,*ofp;
if (argc!=4) {
fprintf(stderr,"usage error\n");
return -1;
}
if (argv[1]=="b") {
b="rb";
}
if (argv[1]=="t") {
b="rt";
}
if (argv[1]!="t"||argv[1]!="b") {
fprintf(stderr,"mode error\n");
return -1;
}
if ((ifp=fopen(argv[2],b))==0) {
fprintf(stderr,"open error i\n");
return -1;
}
if ((ofp=fopen(argv[3],b))==0) {
fprintf(stderr,"open error o\n");
return -1;
}
while(a!=EOF)
a=fgetc(ifp);
fputc(a,ofp);
printf("done\n");
return 0;}
Click to expand...
Is anyone good enough to glance at this and see what's wrong?
Click to expand...

Too many things for a glance. It requires a perusal.

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

int main(int argc, char **argv)
{
char *imode = 0;
char *omode = 0;
char option;
int ch = 0;

FILE *ifp,
*ofp;
if (argc != 4) {
fprintf(stderr, "usage error\n");
return EXIT_FAILURE;
}
option = *argv[1];
if (option == 'b') {
imode = "rb";
omode = "wb";
}
if (option == 't') {
/* This may or may not do something useful on your system */
/* The 't' in the mode is not portable, but is a fairly */
/* popular extension for text mode. */
imode = "rt";
omode = "wt";
Click to expand...
Hahah, what else will the troll come up with? You're entertaining, but
it's sad that posters take you seriously.
 
B

Bill Cunningham

[snip]

I think you want braces around these statements, yes?
Click to expand...

I don't know. I've ran code before and seen a lot of people use while
and statements after it and the code works. No braces. So I usually use it
like that. It's not a typo I don't usually use braces with while.
 
K

Keith Thompson

Bill Cunningham said:
[snip]

I think you want braces around these statements, yes?
Click to expand...

I don't know. I've ran code before and seen a lot of people use while
and statements after it and the code works. No braces. So I usually use it
like that. It's not a typo I don't usually use braces with while.
Click to expand...
[...]

Do you understand what the braces mean?

Here's the tail end of the code Robert was commenting on:

[...]
while(a!=EOF)
a=fgetc(ifp);
fputc(a,ofp);
printf("done\n");
return 0;}

Putting the closing brace immediately after the semicolon like that is
horribly bad style. Putting everything at the same indentation level
is also horribly bad style. The compiler doesn't care, but anyone
trying to read your code does.

The syntax of a while statement is:

while ( expression ) statement

Note that that's a *single* statement. If you want the body to be
multiple statements, you need to wrap them in braces to create a
compound statement, which is itself as single statement. (The same
thing applies to if statements; you correctly used braces for those,
so I don't know why the while loop was such a problem.)

The code I quoted is equivalent to this:

[...]
while (a != EOF)
a = fgetc(ifp);
fputc(a, ofp);
printf("done\n");
return 0;
}

With proper indentation, you can see that the while controls only a
single statement, ``a = fgetc(ifp);''. The fputc call will occur
exactly once, after the while loop terminates. I can't be certain,
but I *think* you wanted both the fgetc and fputc calls to be in the
body of the loop. To do that, you need braces:

[...]
while (a != EOF) {
a = fgetc(ifp);
fputc(a, ofp);
}
printf("done\n");
return 0;
}

In my opinion, it's a good habit always to use braces even if you only
want only a single statement in the body. It makes the code more
consistent and easier to maintain if you later want to add a second
statement. (Others will disagree.)

Judicious use of whitespace, especially around operators and after
commas, also makes the code much easier to read.

From now on, if you post a chunk of nearly illegible code like what
you posted upthread, with no indentation and virtually no whitespace,
I will ask you to format it properly before I'll even consider helping
you fix any other problems.
 
B

Bill Cunningham

The syntax of a while statement is:

while ( expression ) statement

Note that that's a *single* statement. If you want the body to be
multiple statements, you need to wrap them in braces to create a
compound statement,
Click to expand...

I see.

which is itself as single statement. (The same
thing applies to if statements; you correctly used braces for those,
Click to expand...

I think braces are required with if.
so I don't know why the while loop was such a problem.)

The code I quoted is equivalent to this:

[...]
while (a != EOF)
a = fgetc(ifp);
fputc(a, ofp);
printf("done\n");
return 0;
}

With proper indentation, you can see that the while controls only a
single statement, ``a = fgetc(ifp);''. The fputc call will occur
exactly once, after the while loop terminates. I can't be certain,
but I *think* you wanted both the fgetc and fputc calls to be in the
body of the loop. To do that, you need braces:

[...]
while (a != EOF) {
a = fgetc(ifp);
fputc(a, ofp);
}
printf("done\n");
return 0;
}

In my opinion, it's a good habit always to use braces even if you only
want only a single statement in the body. It makes the code more
consistent and easier to maintain if you later want to add a second
statement. (Others will disagree.)

Judicious use of whitespace, especially around operators and after
commas, also makes the code much easier to read.

From now on, if you post a chunk of nearly illegible code like what
you posted upthread, with no indentation and virtually no whitespace,
I will ask you to format it properly before I'll even consider helping
you fix any other problems.
Click to expand...

OK

Bill
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Similar Threads

Please help with C programming to save GPS reception data in Raspberry Pi. 0
Linux: using "clone3" and "waitid" 0
Command Line Arguments 0
Adding adressing of IPv6 to program 1
Compiling a c program 5
Working with files 1
Remove repeated words from a file 3
How can I view / open / render / display a pdf file with c code? 0

Members online

Total: 31 (members: 1, guests: 30)
Robots: 186

Forum statistics

Threads
473,773
Messages
2,569,594
Members
45,123
Latest member
Layne6498

Latest Threads

Top