Processing input from pipes

Discussion in 'C Programming' started by Christopher J. Ruwe, Nov 27, 2010.

  1. Dear list,


    I am a bit confused with a couple of very simple C programs I wrote.

    One program takes a string and transforms each character into a
    corresponding integer, starting with zero (a -> 0, b -> 1, ...).

    The next program transforms a set of integers and applies the Vigenere
    cypher (currently hard-coded).

    The third transforms a set of integers to upper-case chars (0 -> A, 1
    -> B, ...).

    I would like to interconnect these little programs, so that
    (PWD=/home/user/workspace/try) ./integerize verysecrettext | ./vigenere
    | ./characterise, which unfortunately does not work. Creating a named
    Pipe with mkfifo does not work either and ends with a broken pipe.

    I am currently refreshing my C, which is in a very bad shape, so my
    question might be stupid ... can anybody discern from my code what
    might be the problem here?

    Thanks in advance, cheers, Christopher

    integerize.c:

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

    int transform(char c);

    int main(int argc, char *argv[])
    {
    int i; /* C90-style counter variable */
    char c; /* single char passed to transform */

    for(i = 0; i < strlen(argv[1]); i++)
    {
    printf("%3i", transform(argv[1]));
    }

    exit(EXIT_SUCCESS);

    }

    int transform(char c)
    {
    int r;
    switch(c)
    {
    case 'a': r = 0;
    break;
    case 'b': r = 1;
    break;
    case 'c': r = 2;
    break;
    case 'd': r = 3;
    break;
    case 'e': r = 4;
    break;
    case 'f': r = 5;
    break;
    case 'g': r = 6;
    break;
    case 'h': r = 7;
    break;
    case 'i': r = 8;
    break;
    case 'j': r = 9;
    break;
    case 'k': r = 10;
    break;
    case 'l': r = 11;
    break;
    case 'm': r = 12;
    break;
    case 'n': r = 13;
    break;
    case 'o': r = 14;
    break;
    case 'p': r = 15;
    break;
    case 'q': r = 16;
    break;
    case 'r': r = 17;
    break;
    case 's': r = 18;
    break;
    case 't': r = 19;
    break;
    case 'u': r = 20;
    break;
    case 'v': r = 21;
    break;
    case 'w': r = 22;
    break;
    case 'x': r = 23;
    break;
    case 'y': r = 24;
    break;
    case 'z': r = 25;
    break;
    default: r = 99;
    }

    return r;
    }

    vigenere.c:

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

    int main(int argc, char *argv[])
    {
    int i;
    int out[argc-1];
    int key[] = {10, 4, 24};

    for(i = 0; i < argc - 1; i++)
    {
    out = atoi(argv[i+1]);
    }

    for(i = 0; i < argc - 1; i++)
    {
    out = (out + key[i % (sizeof(key)/sizeof(int))]) % 26;
    }

    for(i = 0; i < argc - 1; i++)
    {
    printf("%3i", out);
    }

    exit(EXIT_SUCCESS);
    }


    characterise.c:

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

    char transform(int i);

    int main(int argc, char *argv[])
    {
    int i; /* C90-style counter variable */

    for(i = 1; i < argc; i++)
    {
    printf("%2c", transform(atoi(argv)));
    }

    exit(EXIT_SUCCESS);

    }

    char transform(int i)
    {
    char r;
    switch(i)
    {
    case 0 : r = 'A';
    break;
    case 1 : r = 'B';
    break;
    case 2 : r = 'C';
    break;
    case 3 : r = 'D';
    break;
    case 4 : r = 'E';
    break;
    case 5 : r = 'F';
    break;
    case 6 : r = 'G';
    break;
    case 7 : r = 'H';
    break;
    case 8 : r = 'I';
    break;
    case 9 : r = 'J';
    break;
    case 10 : r = 'K';
    break;
    case 11 : r = 'L';
    break;
    case 12 : r = 'M';
    break;
    case 13 : r = 'N';
    break;
    case 14 : r = 'O';
    break;
    case 15 : r = 'P';
    break;
    case 16 : r = 'Q';
    break;
    case 17 : r = 'R';
    break;
    case 18 : r = 'S';
    break;
    case 19 : r = 'T';
    break;
    case 20 : r = 'U';
    break;
    case 21 : r = 'V';
    break;
    case 22 : r = 'W';
    break;
    case 23 : r = 'X';
    break;
    case 24 : r = 'Y';
    break;
    case 25 : r = 'Z';
    break;
    default: r = '-';
    }

    return r;
    }
    Christopher J. Ruwe, Nov 27, 2010
    #1
    1. Advertising

  2. Christopher J. Ruwe

    Ike Naar Guest

    On 2010-11-27, Christopher J. Ruwe <> wrote:
    > [...]
    > I would like to interconnect these little programs, so that
    > (PWD=/home/user/workspace/try) ./integerize verysecrettext | ./vigenere
    >| ./characterise, which unfortunately does not work.


    In that case, rewrite vigenere.c and characterise.c so that they read
    their input from stdin. The versions you showed take their input
    from the argument list.
    Ike Naar, Nov 27, 2010
    #2
    1. Advertising

  3. Christopher J. Ruwe

    Mark Wooding Guest

    "Christopher J. Ruwe" <> writes:

    > Dear list,


    This isn't a list. It's a Usenet newsgroup. And remember:

    Usenet: the anti-social network.

    > I would like to interconnect these little programs, so that
    > (PWD=/home/user/workspace/try) ./integerize verysecrettext | ./vigenere
    > | ./characterise, which unfortunately does not work. Creating a named
    > Pipe with mkfifo does not work either and ends with a broken pipe.


    Pipes aren't really a C thing; but your problem sort of is, and your
    programs certainly are, so I'll let that slide for now.

    > integerize.c:
    >
    > #include <stdio.h>
    > #include <stdlib.h>
    > #include <string.h>
    >
    > int transform(char c);


    Why have you made `transform' externally visible? I'd mark this as
    `static' here. (In fact, I'd write the body of `transform' here, so I
    don't need to write the argument list twice in two separate places.)

    > int main(int argc, char *argv[])
    > {
    > int i; /* C90-style counter variable */
    > char c; /* single char passed to transform */
    >
    > for(i = 0; i < strlen(argv[1]); i++)


    This has undefined behaviour (likely a segfault) if no argument is
    provided to the program, since `argv[1]' will be a null pointer. If
    this is anything other than a program you'll throw away in half an hour,
    you should check that the arguments are actually present, and report a
    vaguely sensible error:

    if (argc != 2) {
    fprintf(stderr, "Usage: integerise STRING\n");
    exit(EXIT_FAILURE);
    }

    This is going to evaluate `strlen(argv[1])' every iteration, potentially
    (if your compiler isn't clever enough to notice that it doesn't have
    to -- which requires quite a lot of cleverness) counting through every
    character of the string.

    Maybe

    for (i = 0; argv[1]; i++)

    would be better. More idiomatic would be to use a pointer:

    const char *p;

    for (p = argv[1]; *p; p++)

    and use `*p' in place of `argv[1]'.

    > {
    > printf("%3i", transform(argv[1]));
    > }


    Here you assume that `transform' returns an integer which can be written
    in two digits (otherwise they'll run into each other). Since you seem
    to be happy with a leading space, why not write

    printf(" %2d", transform(argv[1]);

    here? (`%d' and `%i' mean the same thing to `printf', but `%d' seems
    more common. They don't mean the same thing to `scanf'.)

    > exit(EXIT_SUCCESS);


    Common error: you're not checking for output errors. Before this, I'd
    write

    if (fflush(stdout) || ferror(stdout) || fclose(stdout)) {
    fprintf(stderr, "error writing output: %s", strerror(errno));
    exit(EXIT_FAILURE);
    }

    One must

    #include <errno.h>

    for `errno'.

    > }
    >
    > int transform(char c)
    > {
    > int r;
    > switch(c)
    > {
    > case 'a': r = 0;
    > break;


    [...]

    > default: r = 99;
    > }
    >
    > return r;
    > }


    Ugh! That's really grim. Some compilers might be clever enough to spot
    the pattern, but I wouldn't bet on it.

    Credit where it's due: you're right not to assume that the letters are
    consecutive or anything like that. This

    if ('a' <= c && c <= 'z') return (c - 'a'); /* BAD */
    else return (99);

    would be nonportable (in particular, it fails on EBCDIC platforms). But
    it is true that the character codes for letters are increasing as you
    progress through the alphabet. We can make use of this by calling
    `bsearch':

    static int chcmp(const void *kv, const void *xv)
    {
    const char *k = kv, *x = xv;

    if (*k < *x) return (-1);
    else if (*k > *x) return (+1);
    else return (0);
    }

    static int transform(char c)
    {
    static const alpha[] = "abcdefghijklmnopqrstuvwxyz";
    const char *p = bsearch(&c, alpha, sizeof(alpha) - 1, 1, chcmp);

    if (p) return (p - alpha);
    else return (99);
    }

    > vigenere.c:
    >
    > #include <stdio.h>
    > #include <stdlib.h>
    >
    > int main(int argc, char *argv[])
    > {
    > int i;
    > int out[argc-1];


    Ooh, a C99 variable-length array. Very slick.

    > int key[] = {10, 4, 24};


    You don't change `key', so you could usefully tell the compiler (a) that
    it's never changed, and (b) that you only need one copy:

    static const int key[] = { 10, 4, 24 };

    >
    > for(i = 0; i < argc - 1; i++)
    > {
    > out = atoi(argv[i+1]);
    > }


    And this is, I think, where you're coming really unstuck. The output of
    the first program was written to stdout. The pipeline `foo | bar' like
    you describe above makes the stdout of `foo' available as stdin to
    `bar'. But you're not reading `stdin': you're looking at your
    comman-line arguments. Unsurprisingly, there's nothing there: you
    didn't write any. This program will ignore its stdin, eventually
    (implicitly) closing it. That's what causes the previous program in the
    pipeline to fail with the `broken pipe' error you report.

    Astonishingly, it might be the case that `scanf' is the best function to
    use for this. See below.

    > for(i = 0; i < argc - 1; i++)
    > {
    > out = (out + key[i % (sizeof(key)/sizeof(int))]) % 26;


    It might be better to say `sizeof(key)/sizeof(key[0])' here, so you
    don't have to do anything special if you change the element type of
    `key'. Indeed, this works for any array, so you could usefully write a
    macro

    #define NMEMB(v) (sizeof(v)/sizeof((v)[0]))

    This will lose the `99' markers that `integerise' output. When you
    decrypt, you'll end up with 99%26 = 21 (`v', I think), which may be
    puzzling.

    > }
    >
    > for(i = 0; i < argc - 1; i++)
    > {
    > printf("%3i", out);
    > }


    (Same remarks about the `printf' format string here.)

    You make three passes over the array, which means writing that tedious
    boilerplate `for' loop three times. It also means that you need the
    intermediate storage. If messing with arrays was the purpose of your
    exercise then that might be plausible; I don't know if that's the case
    or not. Anyway, I'd write something like this instead.

    int i, d;

    while (scanf("%d", &d) > 0) {
    if (d >= 26)
    printf(" %d", d);
    else {
    printf(" %d", (d + key)%26);
    i++;
    if (i > NMEMB(key)) i = 0;
    }
    }

    (I'm not entirely sure whether to step the key index along for errors or
    not. Either seems OK.)

    Now, the above loop might stop for any of three reasons:

    * We ran out of input data. Then `feof(stdin)' will be true.

    * We hit some kind of I/O error while reading. Unlikely for pipes,
    but it's worth checking. Then `ferror(stdin)' will be true.

    * `scanf' found something in the input which it didn't like.

    Something like this.

    if (ferror(stdin)) {
    fprintf(stderr, "error reading input: %s\n", strerror(errno));
    exit(EXIT_FAILURE);
    }
    if (!feof(stdin)) {
    fprintf(stderr, "malformed input\n");
    exit(EXIT_FAILURE);
    }

    and then check for output errors as above.

    > exit(EXIT_SUCCESS);
    > }


    > characterise.c:
    >
    > #include <stdio.h>
    > #include <stdlib.h>
    > #include <string.h>
    >
    > char transform(int i);
    >
    > int main(int argc, char *argv[])
    > {
    > int i; /* C90-style counter variable */
    >
    > for(i = 1; i < argc; i++)
    > {
    > printf("%2c", transform(atoi(argv)));
    > }


    Again, you're iterating over command-line arguments rather than stdin.
    I'll leave fixing this as an exercise.

    > exit(EXIT_SUCCESS);
    >
    > }
    >
    > char transform(int i)
    > {
    > char r;
    > switch(i)
    > {
    > case 0 : r = 'A';
    > break;


    [...]

    > default: r = '-';
    > }
    >
    > return r;
    > }


    Ugh! (Again). This time we can avoid messing with `bsearch' because we
    know the shape of the input space. Arrays were made for this.

    static char transform(int i)
    {
    static const alpha[] = "abcdefghijklmnopqrstuvwxyz";

    if (i >= 0 && i < sizeof(alpha) - 1) return (alpha);
    else return ('-');
    }

    -- [mdw]
    Mark Wooding, Nov 27, 2010
    #3
  4. Christopher J. Ruwe

    Bogdan Guest

    On Nov 27, 7:59 pm, "Christopher J. Ruwe" <> wrote:
    > Dear list,
    >
    > I am a bit confused with a couple of very simple C programs I wrote.
    >
    > One program takes a string and transforms each character into a
    > corresponding integer, starting with zero (a -> 0, b -> 1, ...).
    >
    > The next program transforms a set of integers and applies the Vigenere
    > cypher (currently hard-coded).
    >
    > The third transforms a set of integers to upper-case chars (0 -> A, 1
    > -> B, ...).
    >
    > I would like to interconnect these little programs, so that
    > (PWD=/home/user/workspace/try) ./integerize verysecrettext | ./vigenere
    > | ./characterise, which unfortunately does not work. Creating a named
    > Pipe with mkfifo does not work either and ends with a broken pipe.
    >
    > I am currently refreshing my C, which is in a very bad shape, so my
    > question might be stupid ... can anybody discern from my code what
    > might be  the problem here?
    >
    > Thanks in advance, cheers, Christopher
    >
    > integerize.c:
    >
    > #include <stdio.h>
    > #include <stdlib.h>
    > #include <string.h>
    >
    > int transform(char c);
    >
    > int main(int argc, char *argv[])
    >   {
    >     int i; /* C90-style counter variable */
    >     char c; /* single char passed to transform */
    >
    >     for(i = 0; i < strlen(argv[1]); i++)
    >       {
    >         printf("%3i", transform(argv[1]));
    >       }
    >
    >     exit(EXIT_SUCCESS);
    >
    >   }
    >
    > int transform(char c)
    >   {
    >     int r;
    >     switch(c)
    >       {
    >         case 'a': r = 0;
    >           break;
    >         case 'b': r = 1;
    >           break;
    >         case 'c': r = 2;
    >           break;
    >         case 'd': r = 3;
    >           break;
    >         case 'e': r = 4;
    >           break;
    >         case 'f': r = 5;
    >           break;
    >         case 'g': r = 6;
    >           break;
    >         case 'h': r = 7;
    >           break;
    >         case 'i': r = 8;
    >           break;
    >         case 'j': r = 9;
    >           break;
    >         case 'k': r = 10;
    >           break;
    >         case 'l': r = 11;
    >           break;
    >         case 'm': r = 12;
    >           break;
    >         case 'n': r = 13;
    >           break;
    >         case 'o': r = 14;
    >           break;
    >         case 'p': r = 15;
    >           break;
    >         case 'q': r = 16;
    >           break;
    >         case 'r': r = 17;
    >           break;
    >         case 's': r = 18;
    >           break;
    >         case 't': r = 19;
    >           break;
    >         case 'u': r = 20;
    >           break;
    >         case 'v': r = 21;
    >           break;
    >         case 'w': r = 22;
    >           break;
    >         case 'x': r = 23;
    >           break;
    >         case 'y': r = 24;
    >           break;
    >         case 'z': r = 25;
    >           break;
    >         default:  r = 99;
    >       }
    >
    >     return r;
    >   }
    >
    > vigenere.c:
    >
    > #include <stdio.h>
    > #include <stdlib.h>
    >
    > int main(int argc, char *argv[])
    >   {
    >     int i;
    >     int out[argc-1];
    >     int key[] = {10, 4, 24};
    >
    >     for(i = 0; i < argc - 1; i++)
    >       {
    >         out = atoi(argv[i+1]);
    >       }
    >
    >     for(i = 0; i < argc - 1; i++)
    >       {
    >         out = (out + key[i % (sizeof(key)/sizeof(int))]) % 26;
    >       }
    >
    >     for(i = 0; i < argc - 1; i++)
    >       {
    >         printf("%3i", out);
    >       }
    >
    >     exit(EXIT_SUCCESS);
    >   }
    >
    > characterise.c:
    >
    > #include <stdio.h>
    > #include <stdlib.h>
    > #include <string.h>
    >
    > char transform(int i);
    >
    > int main(int argc, char *argv[])
    >   {
    >     int i; /* C90-style counter variable */
    >
    >     for(i = 1; i < argc; i++)
    >       {
    >         printf("%2c", transform(atoi(argv)));
    >       }
    >
    >     exit(EXIT_SUCCESS);
    >
    >   }
    >
    > char transform(int i)
    >   {
    >     char r;
    >     switch(i)
    >       {
    >         case  0 : r = 'A';
    >           break;
    >         case  1 : r = 'B';
    >           break;
    >         case  2 : r = 'C';
    >           break;
    >         case  3 : r = 'D';
    >           break;
    >         case  4 : r = 'E';
    >           break;
    >         case  5 : r = 'F';
    >           break;
    >         case  6 : r = 'G';
    >           break;
    >         case  7 : r = 'H';
    >           break;
    >         case  8 : r = 'I';
    >           break;
    >         case  9 : r = 'J';
    >           break;
    >         case 10 : r = 'K';
    >           break;
    >         case 11 : r = 'L';
    >           break;
    >         case 12 : r = 'M';
    >           break;
    >         case 13 : r = 'N';
    >           break;
    >         case 14 : r = 'O';
    >           break;
    >         case 15 : r = 'P';
    >           break;
    >         case 16 : r = 'Q';
    >           break;
    >         case 17 : r = 'R';
    >           break;
    >         case 18 : r = 'S';
    >           break;
    >         case 19 : r = 'T';
    >           break;
    >         case 20 : r = 'U';
    >           break;
    >         case 21 : r = 'V';
    >           break;
    >         case 22 : r = 'W';
    >           break;
    >         case 23 : r = 'X';
    >           break;
    >         case 24 : r = 'Y';
    >           break;
    >         case 25 : r = 'Z';
    >           break;
    >         default:  r = '-';
    >       }
    >
    >     return r;
    >   }


    Besides the fact that you should use stdin/stdout (such a program is
    called a filter), you should also bear in mind that printf buffers its
    output, so you should call immediately fflush(stdout). A better
    solution would be to use read()/write() functions.
    Bogdan, Nov 28, 2010
    #4
  5. Christopher J. Ruwe

    BartC Guest

    "Mark Wooding" <> wrote in message
    news:...
    > "Christopher J. Ruwe" <> writes:


    >> int r;
    >> switch(c)
    >> {
    >> case 'a': r = 0;
    >> break;

    >
    > [...]
    >
    >> default: r = 99;
    >> }
    >>
    >> return r;
    >> }

    >
    > Ugh! That's really grim. Some compilers might be clever enough to spot
    > the pattern, but I wouldn't bet on it.
    >
    > Credit where it's due: you're right not to assume that the letters are
    > consecutive or anything like that. This
    >
    > if ('a' <= c && c <= 'z') return (c - 'a'); /* BAD */
    > else return (99);
    >
    > would be nonportable (in particular, it fails on EBCDIC platforms). But
    > it is true that the character codes for letters are increasing as you
    > progress through the alphabet. We can make use of this by calling
    > `bsearch':


    This could be an unnecessary overhead, given the 99% probability that the
    platform is going to be ASCII or UTF-8. What about:

    if ('A'==65) /* assume ASCII */
    /* 'bad' way */
    else
    /* bsearch stuff */

    And quite likely the bsearch branch won't even be compiled.

    --
    Bartc
    BartC, Nov 28, 2010
    #5
  6. Christopher J. Ruwe

    Mark Wooding Guest

    "BartC" <> writes:

    > This could be an unnecessary overhead, given the 99% probability that the
    > platform is going to be ASCII or UTF-8. What about:
    >
    > if ('A'==65) /* assume ASCII */


    Better: if ('z' == 'a' + 25) ...

    You could also use the preprocessor if you didn't want to rely on the
    compiler being sufficiently clever.

    -- [mdw]
    Mark Wooding, Nov 28, 2010
    #6
  7. Thank you all very much for your help, especially @Mark Wooding for your
    really extensive correction.

    It took me some time to really understand how to properly iterate in a
    ring in the ` while(scanf("%d", &d) > 0) { ... } ' (much more elegant,
    though), so I will take the exercise on to next weekend.

    Again, thank you very much, cheers
    --
    Christopher J. Ruwe
    TZ GMT + 1
    Christopher J. Ruwe, Nov 28, 2010
    #7
    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. piyush
    Replies:
    0
    Views:
    1,844
    piyush
    Jul 14, 2004
  2. Brian Hann

    IPC::Run and hanging pipes

    Brian Hann, Dec 8, 2003, in forum: Perl
    Replies:
    1
    Views:
    787
    Brian Hann
    Dec 11, 2003
  3. Hubert Hung-Hsien Chang
    Replies:
    2
    Views:
    451
    Michael Foord
    Sep 17, 2004
  4. Alex
    Replies:
    0
    Views:
    377
  5. Narayanan K

    Input/Output to IRB using Pipes

    Narayanan K, Jun 16, 2010, in forum: Ruby
    Replies:
    2
    Views:
    106
    Hidetoshi NAGAI
    Jun 17, 2010
Loading...

Share This Page