Re: [Second patch] misc-utils: Re-write scriptreplay in C.

["James Youngman" <jay@xxxxxxx>]

On Wed, Apr 9, 2008 at 10:46 AM, Karel Zak <kzak@xxxxxxxxxx> wrote:
>
>   Well, it seems we have worked on the same thing:
>   http://thread.gmane.org/gmane.linux.utilities.util-linux-ng/1371
>
>   So... see the patch below. It's based on both patches.

Well first, sorry about the collision and extra work.      I only
subscribed after making my patches.   My mistake.

I like the patch you just posted.   I only have two comments:
1. The use of the variable name "fd" for a FILE* is a little confusing
to me, as I would normally expect "fd" to stand for "file descriptor"
and be a small integer returned by open(2), dup(2) etc.
2. Just putting "\n" in the scan format is not enough to verify that a
\n is actually in the input.  See this example program:


orbital:misc$ cat newline-scan.c
#include <stdio.h>

int main (int argc, char *argv[]) {
  int a, b, nf;

  for (;;) {
    nf = scanf("%d %d\n", &a, &b);
    if (EOF == nf) {
      break;
    } else if (nf != 2) {
      fprintf(stderr, "Only converted %d fields\n", nf);
      return 1;
    } else {
      printf("a=%d, b=%d\n", a, b);
    }
  }
  return 0;
}
orbital:misc$ gcc -Wall newline-scan.c -o newline-scan
orbital:misc$ echo 1 2 3 4  | ./newline-scan
a=1, b=2
a=3, b=4

The intent of my messing around with the %[ format directive and
checking the "newline" variable was to ensure that there were only two
fields per line; in other words that the format of the input is
correct.  However, your version will, I think, accept input files
which don't have newlines in the expected place.  That could be a
useful feature for people who want to be able to edit the timings file
for example.  Anyway, I think which form of the code one prefers is
largely a matter of how much validation one believes is useful here.
I'm happy to leave that choice up to you.

Thanks,
James.
--
To unsubscribe from this list: send the line "unsubscribe util-linux-ng" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html