Processing input from pipes

  • Thread starter Christopher J. Ruwe
  • Start date
C

Christopher J. Ruwe

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

Ike Naar

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

Mark Wooding

Christopher J. Ruwe said:
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]
 
B

Bogdan

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.
 
B

BartC

Mark Wooding said:
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.
 
M

Mark Wooding

BartC said:
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]
 
C

Christopher J. Ruwe

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
 

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

Members online

No members online now.

Forum statistics

Threads
473,769
Messages
2,569,578
Members
45,052
Latest member
LucyCarper

Latest Threads

Top