neater code

Discussion in 'C Programming' started by Bill Cunningham, Jan 31, 2010.

  1. 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);
    }
     
    Bill Cunningham, Jan 31, 2010
    #1
    1. Advertising

  2. Bill Cunningham

    Chad Guest

    On Jan 31, 9:59 am, "Bill Cunningham" <> 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?
     
    Chad, Jan 31, 2010
    #2
    1. Advertising

  3. Bill Cunningham

    Lew Pitcher Guest

    On January 31, 2010 12:59, in comp.lang.c, 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. ------
     
    Lew Pitcher, Jan 31, 2010
    #3
  4. 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;
    }
     
    Andrea Laforgia, Jan 31, 2010
    #4
  5. "Bill Cunningham" <> 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) <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"
     
    Keith Thompson, Jan 31, 2010
    #5
  6. On Jan 31, 7:59 pm, "Bill Cunningham" <> 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.
     
    Michael Foukarakis, Feb 1, 2010
    #6
  7. On Sun, 31 Jan 2010 22:31:02 +0100, Andrea Laforgia
    <> 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
     
    Barry Schwarz, Feb 1, 2010
    #7
  8. "Michael Foukarakis" <> wrote in message
    news:...

    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
     
    Bill Cunningham, Feb 1, 2010
    #8
  9. Bill Cunningham

    santosh Guest

    Bill Cunningham wrote:
    > "Michael Foukarakis" <> wrote in message
    > news:...
    >
    > 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.
     
    santosh, Feb 1, 2010
    #9
  10. "Bill Cunningham" <> writes:
    > "Michael Foukarakis" <> wrote in message
    > news:...

    [...]
    > 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) <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"
     
    Keith Thompson, Feb 1, 2010
    #10
  11. On Mon, 01 Feb 2010 02:46:45 -0800, Barry Schwarz <>
    wrote:

    >How does reportTheError tell the difference between the input file and
    >the output file?


    Correct. I guess closeFile() is not a very good idea :)

    > How does closeFile inform the calling function that
    >an error occurred.?


    Well, I didn't think it should. I thought of it as a replacement of
    the group of lines attempting to close the file and reporting the
    error. Instead of repeating those lines, one could use closeFile(),
    but I realize that one should also pass a good bunch of information
    together with the file pointer, in order to have a reliable error log,
    which might make this function very little worth using :)

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

    >
    >Why not use a portable return value?


    Right: EXIT_FAILURE. I simply copied the initial approach.
     
    Andrea Laforgia, Feb 1, 2010
    #11
  12. On Feb 1, 8:21 pm, Keith Thompson <> wrote:
    > "Bill Cunningham" <> writes:
    > > "Michael Foukarakis" <> wrote in message
    > >news:....

    > [...]
    > >     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.


    But I wasn't trying to do that, I was simply going for a neater
    version of its code. It turned out that lots of functionality could be
    stripped leading to a much neater, visually speaking, program.
     
    Michael Foukarakis, Feb 2, 2010
    #12
  13. On 31 Jan, 21:31, Andrea Laforgia <>
    wrote:

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


    I doubt it! Richard heathfield is a structured programmer of the old
    school.

    "programs shall be written using only the constructs sequence,
    iteration and conditional"
    "each code block and function shall have a single entry point and a
    single exit point"

    no gotos no early exits


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


    good grief. You are suggesting Richard should invoke implementaion
    defined behaviour when there is a well defined portable solution?!

    :)


    > }
    >
    > 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;
    >
    >
    >
    > }




    --
    It's probably not the quickest method around, but it uses plenty of
    trees, and that's what counts.
    Richard Heathfield
     
    Nick Keighley, Feb 2, 2010
    #13
  14. Bill Cunningham

    _JusSx_ Guest

    On 2010-01-31, Bill Cunningham <> 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);
    > }
    >
    >


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


    int
    main (argc, argv)
    int argc;
    char *argv[];
    {
    int c;

    while ((c = fgetc(stdin)) != EOF)
    fputc(~c, stdout);

    return EXIT_SUCCESS;
    }
    #v-

    do you like this way?

    -jussx-

    --
    Linux is only free if your time has no value
     
    _JusSx_, Mar 1, 2010
    #14
  15. Bill Cunningham

    Ian Collins Guest

    _JusSx_ wrote:
    >
    > #v+


    Eh?
    > #include <stdio.h>
    > #include <stdlib.h>
    >
    >
    > int
    > main (argc, argv)
    > int argc;
    > char *argv[];


    Yuck, that's ancient K&R style. Avoid at all costs!

    --
    Ian Collins
     
    Ian Collins, Mar 1, 2010
    #15
    1. Advertising

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

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. =?Utf-8?B?Q2FybG8gTWFyY2hlc29uaQ==?=

    Fire Code behind code AND Javascript code associated to a Button Click Event

    =?Utf-8?B?Q2FybG8gTWFyY2hlc29uaQ==?=, Feb 10, 2004, in forum: ASP .Net
    Replies:
    4
    Views:
    21,245
    =?Utf-8?B?Q2FybG8gTWFyY2hlc29uaQ==?=
    Feb 11, 2004
  2. Stimp
    Replies:
    5
    Views:
    474
    Stimp
    Oct 14, 2005
  3. keithb
    Replies:
    1
    Views:
    918
    Bruce Barker
    Mar 29, 2006
  4. Ross Clement (Email address invalid - do not use)

    Neater method of creating a linked list

    Ross Clement (Email address invalid - do not use), Nov 30, 2005, in forum: C Programming
    Replies:
    2
    Views:
    280
    Richard Harnden
    Nov 30, 2005
  5. Replies:
    9
    Views:
    102
    Brad Baxter
    May 1, 2006
Loading...

Share This Page