Velocity Reviews - Computer Hardware Reviews

Velocity Reviews > Newsgroups > Programming > C Programming > neater code

Reply
Thread Tools

neater code

 
 
Bill Cunningham
Guest
Posts: n/a
 
      01-31-2010
I wrote this small program to take an ELF format file and turn it inside
out. Turn all off bits on and vice versa. I think this code can be a little
neater and I found that a 2Kb file became 2 bytes. I don't know if the
system is reading the file data wrong because everything has been "flipped"
or what. I want to be able to flip everything back but I might have in doing
this lost data. Could this file be made to look a little neater with maybe a
while or two instead of the IF's ? I tried it and failed. Any suggestions?

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

int main(int argc, char **argv)
{
FILE *fp, *fp0;
int o = 0;
int i = 0;
int fl = ~o;
if (argc != 3) {
fputs("flip usage error\n", stderr);
exit(1);
}
fp = fopen(argv[1], "rb");
fp0 = fopen(argv[2], "wb");
if ((i = getc(fp)) != EOF)
i = getc(fp);
if ((o = putc(fl, fp0)) != EOF)
o = putc(fl, fp0);

fclose(fp);
fclose(fp0);
exit(0);
}


 
Reply With Quote
 
 
 
 
Chad
Guest
Posts: n/a
 
      01-31-2010
On Jan 31, 9:59*am, "Bill Cunningham" <(E-Mail Removed)> wrote:
> * * I wrote this small program to take an ELF format file and turn it inside
> out. Turn all off bits on and vice versa. I think this code can be a little
> neater and I found that a 2Kb file became 2 bytes. I don't know if the
> system is reading the file data wrong because everything has been "flipped"
> or what. I want to be able to flip everything back but I might have in doing
> this lost data. Could this file be made to look a little neater with maybe a
> while or two instead of the IF's ? I tried it and failed. Any suggestions?
>
> #include <stdio.h>
> #include <stdlib.h>
>
> int main(int argc, char **argv)
> {
> * * FILE *fp, *fp0;
> * * int o = 0;
> * * int i = 0;
> * * int fl = ~o;
> * * if (argc != 3) {
> * * * * fputs("flip usage error\n", stderr);
> * * * * exit(1);
> * * }
> * * fp = fopen(argv[1], "rb");
> * * fp0 = fopen(argv[2], "wb");
> * * if ((i = getc(fp)) != EOF)
> * * * * i = getc(fp);
> * * if ((o = putc(fl, fp0)) != EOF)
> * * * * o = putc(fl, fp0);
>
> * * fclose(fp);
> * * fclose(fp0);
> * * exit(0);
>
>
>
> }


ELF? You mean the men that wear green tights and help Santa Claus?
 
Reply With Quote
 
 
 
 
Lew Pitcher
Guest
Posts: n/a
 
      01-31-2010
On January 31, 2010 12:59, in comp.lang.c, http://www.velocityreviews.com/forums/(E-Mail Removed)lid wrote:

> I wrote this small program to take an ELF format file and turn it
> inside out.


Actually, ELF format has nothing to do with it. Your program doesn't depend
on an ELF input at all.

> Turn all off bits on and vice versa.


If that was the intent, then you didn't think it through very well. You see,
that's /not/ what your program does, not even close.

> I think this code can be a little neater


Likely /All/ programs look untidy

> and I found that a 2Kb file became 2 bytes.


Understandable, given your program.

> I don't know if the system is reading the file data wrong because
> everything has been "flipped" or what.


No. You just wrote a 2 byte file. That's what your logic explicitly does. It
does not "flip bits" or transform an input file

> I want to be able to flip everything back but I might have in doing this
> lost data.


So long as you kept the original input file, you should be OK. If you ran
your program with input and output files being the same file, then you
killed yourself. The input file cannot be recovered from the output file.

> Could this file be made to look a little neater with maybe a while or two
> instead of the IF's ?


Very certainly

> I tried it and failed. Any suggestions?


Let's look at your code, shall we?

> #include <stdio.h>
> #include <stdlib.h>
>
> int main(int argc, char **argv)
> {
> FILE *fp, *fp0;
> int o = 0;
> int i = 0;
> int fl = ~o;


Since /o/ is already set to 0, then /fl/ is now set to ~0, or "all bits 1"


> if (argc != 3) {
> fputs("flip usage error\n", stderr);
> exit(1);
> }


OK. Adequate argument checking for a sample program.


> fp = fopen(argv[1], "rb");


What if the file named by argv[1] doesn't exist? Will fopen() return a
valid FILE*? What should your program do if the input file doesn't exist?
Should it continue, create the output file, and attempt to process it's
non-existant input?


> fp0 = fopen(argv[2], "wb");


What if the file named by argv[2] is the same as the file named by argv[1]?
A "w" mode will cause fopen() to truncate the named file, and that'll
delete any data in it. If argv[2] names the same file as argv[1], then
you've just killed all your input. What other situations might you want to
cover here? What if fopen() returns an error?


> if ((i = getc(fp)) != EOF)


You read a character from the input file,
save it in /i/,
test it to see if it equals the EOF indicator, and
execute the next line if it doesn't.

That's one byte read out of your input file


> i = getc(fp);


(If the first input byte was not EOF),
you read a (second) character from the input and save it in /i/, discarding
the first input byte.

That's now two bytes read from the input file.


> if ((o = putc(fl, fp0)) != EOF)

You write one byte of ~0 fron /fl/ to the output file,
save the results of the write (which can be either your original ~0 or
EOF) in /o/,
test the results to see if it equals the EOF indicator, and
execute the next line if the results were not EOF

That's one byte of ~0 written to your output file.


> o = putc(fl, fp0);

(If the first write succeeded)
you write a second byte of ~0 from /fl/ to the output file, saving the
results of the putc() in /o/ (discarding the previous value of /o/)

That's now two bytes written to the output file.


> fclose(fp);


You close the input file, after reading only two bytes.


> fclose(fp0);


You close the output file, after writing only two bytes.


> exit(0);


You terminate the execution of your program, with a returncode of 0


> }


Notice in the above analysis...

1) You don't read the entire input file. You only read two bytes from the
input.

2) You don't write any of the input (transformed or not) to the output file.
You write only two bytes (at most) of constant data, unrelated to the
contents of the input file.

3) You don't do any transformation on the data obtained from the input file.

4) You don't prevent your program from deleting all the contents of the
input file before processing it

--------------------------------------------------------------

For processing /all/ the input, you need a looping mechanism.

The body of the loop should perform your "complement" transformation on the
input byte, and then write the byte to the output. The loop control itself
should obtain the next input byte, and terminate when the input is EOF.

You want something like ...

while ((i = getc(fp)) != EOF) /* get a byte into /i/, stop on EOF */
{
fl = ~i; /* /fl/ is bitflipped /i/ */
if ((o = putc(fl, fp0)) == EOF) /* write /fl/ to output file */
{ /* if write returned EOF */
fputs("flip failed to write", stderr); /* then we have an error */
exit(2);
}
}

HTH
--
Lew Pitcher
Master Codewright & JOAT-in-training | Registered Linux User #112576
Me: http://pitcher.digitalfreehold.ca/ | Just Linux: http://justlinux.ca/
---------- Slackware - Because I know what I'm doing. ------


 
Reply With Quote
 
Andrea Laforgia
Guest
Posts: n/a
 
      01-31-2010

I agree that he should report all the errors; don't you think that
something like the following would be more readable?

void closeFile(FILE *fp) {
if (fclose(fp)!=0) {
reportTheError(...);
}
}

void exitWithError(...) {
...
exit(1);
}

void reportTheError(...) {
...
}

int main(int argc, char **argv) {

FILE *fpi, *fpo;
unsigned char c;

if (argc != 3)
exitWithError(...);

if (!(fpi = fopen(argv[1], "rb"))
exitWithError(...);

if (!(fpo = fopen(argv[2], "wb")) {
closeFile(fpi);
exitWithError(...);
}

while (fread(&c, 1, 1, fpi)==1) {
unsigned char not_c = ~c;
if (fwrite(&not_c, 1, 1, fpo)!=1) {
reportTheError(...);
break;
}
}

if (ferror(fpi)) {
reportTheError(...);
}

closeFile(fpo);
closeFile(fpi);

return 0;
}
 
Reply With Quote
 
Keith Thompson
Guest
Posts: n/a
 
      01-31-2010
"Bill Cunningham" <(E-Mail Removed)> writes:
> I wrote this small program to take an ELF format file and turn it inside
> out. Turn all off bits on and vice versa. I think this code can be a little
> neater and I found that a 2Kb file became 2 bytes. I don't know if the
> system is reading the file data wrong because everything has been "flipped"
> or what. I want to be able to flip everything back but I might have in doing
> this lost data. Could this file be made to look a little neater with maybe a
> while or two instead of the IF's ? I tried it and failed. Any suggestions?
>
> #include <stdio.h>
> #include <stdlib.h>
>
> int main(int argc, char **argv)
> {
> FILE *fp, *fp0;
> int o = 0;
> int i = 0;
> int fl = ~o;
> if (argc != 3) {
> fputs("flip usage error\n", stderr);
> exit(1);
> }
> fp = fopen(argv[1], "rb");
> fp0 = fopen(argv[2], "wb");
> if ((i = getc(fp)) != EOF)
> i = getc(fp);
> if ((o = putc(fl, fp0)) != EOF)
> o = putc(fl, fp0);
>
> fclose(fp);
> fclose(fp0);
> exit(0);
> }


The real problem is that you think the problem with your code is
neatness rather than correctness.

A while loop is not "neater" than an if statement; it's a different
statement that does a very different thing. You don't choose to
use one or the other based on esthetics; you choose based on which
one does the job you want to do.

Your problem description is "to take an ELF format file and turn
it inside out. Turn all off bits on and vice versa."

What does the format of the input file (ELF or otherwise) have to
do with with this? ELF is an object file format. Inverting all
the bits in an ELF file gives you garbage.

Assuming you change "an ELF format file" to "a file", you're
left with something that might be a reasonable problem statement.
There might even be good reasons to generate a bit-flipped copy of
a given file; it might be a quick and dirty way to render a file
unusable while letting it be easily restored by inverting it again.
And as a simple programming exercise, it's not bad.

The big problem is that the program you posted doesn't do anything
resembling what you described.

The only value you ever write to the output file is fl. The value
of fl never changes after it's been initialized, and you only write
it (at most) twice.

There must have been a long chain of misconceptions leading
to the code you posted. One link in that chain was probably
a misunderstanding of the difference between "if" and "while".
Another *might* have been an assumption that, once you initialize
fl to ~o, it will remain equal to ~o as the value of o changes,
but that's only a guess on my part.

You're doing the equivalent of asking a driving instructor whether
you're using your turn signals properly, when the real problem is
that you need to stop driving the wrong way on the sidewalk.

--
Keith Thompson (The_Other_Keith) (E-Mail Removed) <http://www.ghoti.net/~kst>
Nokia
"We must do something. This is something. Therefore, we must do this."
-- Antony Jay and Jonathan Lynn, "Yes Minister"
 
Reply With Quote
 
Michael Foukarakis
Guest
Posts: n/a
 
      02-01-2010
On Jan 31, 7:59*pm, "Bill Cunningham" <(E-Mail Removed)> wrote:
> * * I wrote this small program to take an ELF format file and turn it inside
> out. Turn all off bits on and vice versa. I think this code can be a little
> neater and I found that a 2Kb file became 2 bytes. I don't know if the
> system is reading the file data wrong because everything has been "flipped"
> or what. I want to be able to flip everything back but I might have in doing
> this lost data. Could this file be made to look a little neater with maybe a
> while or two instead of the IF's ? I tried it and failed. Any suggestions?
>
> #include <stdio.h>
> #include <stdlib.h>
>
> int main(int argc, char **argv)
> {
> * * FILE *fp, *fp0;
> * * int o = 0;
> * * int i = 0;
> * * int fl = ~o;
> * * if (argc != 3) {
> * * * * fputs("flip usage error\n", stderr);
> * * * * exit(1);
> * * }
> * * fp = fopen(argv[1], "rb");
> * * fp0 = fopen(argv[2], "wb");
> * * if ((i = getc(fp)) != EOF)
> * * * * i = getc(fp);
> * * if ((o = putc(fl, fp0)) != EOF)
> * * * * o = putc(fl, fp0);
>
> * * fclose(fp);
> * * fclose(fp0);
> * * exit(0);
>
> }
>

I tried rewriting your program in a neater way, here's what I came up
with:

#include <stdio.h> /* Skip unnecessary includes for brevity */

int main(int argc, char **argv) /* to hell with the brevity */
{
while(printf("Hello world!\n") < 0)
printf("Error greeting world, please advise!\n");
return(~0); /* Flipping bits since ANSI C */
}

You can clearly see the part where the bits are reversed, the program
also prints a greeting for general awesomeness.
 
Reply With Quote
 
Barry Schwarz
Guest
Posts: n/a
 
      02-01-2010
On Sun, 31 Jan 2010 22:31:02 +0100, Andrea Laforgia
<(E-Mail Removed)> wrote:

>
>I agree that he should report all the errors; don't you think that
>something like the following would be more readable?
>
>void closeFile(FILE *fp) {
> if (fclose(fp)!=0) {
> reportTheError(...);


How does reportTheError tell the difference between the input file and
the output file? How does closeFile inform the calling function that
an error occurred.?

> }
>}
>
>void exitWithError(...) {
> ...
> exit(1);


Why not use a portable return value?

>}
>
>void reportTheError(...) {
> ...
>}
>
>int main(int argc, char **argv) {
>
> FILE *fpi, *fpo;
> unsigned char c;
>
> if (argc != 3)
> exitWithError(...);
>
> if (!(fpi = fopen(argv[1], "rb"))
> exitWithError(...);
>
> if (!(fpo = fopen(argv[2], "wb")) {
> closeFile(fpi);
> exitWithError(...);
> }
>
> while (fread(&c, 1, 1, fpi)==1) {
> unsigned char not_c = ~c;
> if (fwrite(&not_c, 1, 1, fpo)!=1) {
> reportTheError(...);
> break;
> }
> }
>
> if (ferror(fpi)) {
> reportTheError(...);
> }
>
> closeFile(fpo);
> closeFile(fpi);
>
> return 0;
>}


--
Remove del for email
 
Reply With Quote
 
Bill Cunningham
Guest
Posts: n/a
 
      02-01-2010

"Michael Foukarakis" <(E-Mail Removed)> wrote in message
news:(E-Mail Removed)...

I tried rewriting your program in a neater way, here's what I came up
with:

#include <stdio.h> /* Skip unnecessary includes for brevity */

int main(int argc, char **argv) /* to hell with the brevity */
{
while(printf("Hello world!\n") < 0)
printf("Error greeting world, please advise!\n");
return(~0); /* Flipping bits since ANSI C */
}

You can clearly see the part where the bits are reversed, the program
also prints a greeting for general awesomeness.

Geesh that's much shorter. A simple return is all that's needed?

Bill


 
Reply With Quote
 
santosh
Guest
Posts: n/a
 
      02-01-2010

Bill Cunningham wrote:
> "Michael Foukarakis" <(E-Mail Removed)> wrote in message
> news:(E-Mail Removed)...
>
> I tried rewriting your program in a neater way, here's what I came up
> with:
>
> #include <stdio.h> /* Skip unnecessary includes for brevity */
>
> int main(int argc, char **argv) /* to hell with the brevity */
> {
> while(printf("Hello world!\n") < 0)
> printf("Error greeting world, please advise!\n");
> return(~0); /* Flipping bits since ANSI C */
> }
>
> You can clearly see the part where the bits are reversed, the program
> also prints a greeting for general awesomeness.
>
> Geesh that's much shorter. A simple return is all that's needed?


No. See Richard's post.
 
Reply With Quote
 
Keith Thompson
Guest
Posts: n/a
 
      02-01-2010
"Bill Cunningham" <(E-Mail Removed)> writes:
> "Michael Foukarakis" <(E-Mail Removed)> wrote in message
> news:(E-Mail Removed)...

[...]
> return(~0); /* Flipping bits since ANSI C */
> }
>
> You can clearly see the part where the bits are reversed, the program
> also prints a greeting for general awesomeness.
>
> Geesh that's much shorter. A simple return is all that's needed?


It depends on what you're trying to do. For what you originally
claimed you were trying to do, it's laughably insufficient.

--
Keith Thompson (The_Other_Keith) (E-Mail Removed) <http://www.ghoti.net/~kst>
Nokia
"We must do something. This is something. Therefore, we must do this."
-- Antony Jay and Jonathan Lynn, "Yes Minister"
 
Reply With Quote
 
 
 
Reply

Thread Tools

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is On
HTML code is Off
Trackbacks are On
Pingbacks are On
Refbacks are Off


Similar Threads
Thread Thread Starter Forum Replies Last Post
Neater way of checking / populating a hash required niall.macpherson@ntlworld.com Perl Misc 9 05-01-2006 07:17 PM
what is the difference between code inside a <script> tag and code in the code-behind file? keithb ASP .Net 1 03-29-2006 01:00 AM
Neater method of creating a linked list Ross Clement (Email address invalid - do not use) C Programming 2 11-30-2005 12:47 PM
what's a neater way of writing this simple code... Stimp ASP .Net 5 10-14-2005 03:03 PM
Fire Code behind code AND Javascript code associated to a Button Click Event =?Utf-8?B?Q2FybG8gTWFyY2hlc29uaQ==?= ASP .Net 4 02-11-2004 07:31 AM



Advertisments