[PATCH] scriptreplay: determine if script --quiet was used to create typescript

[PATCH] scriptreplay: determine if script --quiet was used to create typescript

[Sami Kerola]

Recent commit that removed header timestamp from typescript output when
--quiet option is defined broke scriptreplay.  Trouble was that scriptreplay
always skipped first line of the typescript.  But --quiet makes that line to
be part of what must be printed by scriptreplay.

Initially I thought it could be possible to determine if first line is
header by searching 'Script started on', but not long later realization that
will be difficult due translations lead to try something else.  If timing
file chunks together make exactly the same size as typescript there is no
header.

Reference: 493548b85d528bb13e72af8240fd997fdbfdd7ea
CC: Rui Zhao (renyuneyun) <renyuneyun@gmail.com>
Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 term-utils/scriptreplay.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/term-utils/scriptreplay.c b/term-utils/scriptreplay.c
index 5fd3ecb16..365b73de4 100644
--- a/term-utils/scriptreplay.c
+++ b/term-utils/scriptreplay.c
@@ -25,6 +25,7 @@
 #include <limits.h>
 #include <math.h>
 #include <sys/select.h>
+#include <sys/stat.h>
 #include <unistd.h>
 #include <getopt.h>
 
@@ -58,6 +59,25 @@ usage(FILE *out)
 	exit(out == stderr ? EXIT_FAILURE : EXIT_SUCCESS);
 }
 
+static int
+sfile_has_header(FILE *sfile, FILE *tfile)
+{
+	double delay;
+	size_t blk, total = 0;
+	char nl;
+	struct stat st;
+
+	if (fstat(fileno(sfile), &st) < 0)
+		return 0;
+	while (fscanf(tfile, "%lf %zu%c\n", &delay, &blk, &nl) == 3)
+		total += blk;
+	fseek(sfile, 0, SEEK_SET);
+	fseek(tfile, 0, SEEK_SET);
+	if ((size_t)st.st_size <= total)
+		return 0;
+	return 1;
+}
+
 static double
 getnum(const char *s)
 {
@@ -199,8 +219,8 @@ main(int argc, char *argv[])
 	if (!sfile)
 		err(EXIT_FAILURE, _("cannot open %s"), sname);
 
-	/* ignore the first typescript line */
-	while((c = fgetc(sfile)) != EOF && c != '\n');
+	if (sfile_has_header(sfile, tfile))
+		 while((c = fgetc(sfile)) != EOF && c != '\n');
 
 	for(line = 1; ; line++) {
 		double delay;
-- 
2.12.2

Re: [PATCH] scriptreplay: determine if script --quiet was used to create typescript

[Sami Kerola]

On 14 April 2017 at 21:39, Sami Kerola <kerolasa@iki.fi> wrote:
> diff --git a/term-utils/scriptreplay.c b/term-utils/scriptreplay.c
> index 5fd3ecb16..365b73de4 100644
> --- a/term-utils/scriptreplay.c
> +++ b/term-utils/scriptreplay.c
> @@ -58,6 +59,25 @@ usage(FILE *out)
>         exit(out == stderr ? EXIT_FAILURE : EXIT_SUCCESS);
>  }
>
> +static int
> +sfile_has_header(FILE *sfile, FILE *tfile)
> +{
> +       double delay;
> +       size_t blk, total = 0;
> +       char nl;
> +       struct stat st;
> +
> +       if (fstat(fileno(sfile), &st) < 0)
> +               return 0;
> +       while (fscanf(tfile, "%lf %zu%c\n", &delay, &blk, &nl) == 3)
> +               total += blk;
> +       fseek(sfile, 0, SEEK_SET);

sfile file position does not need updating, fixed in:

https://github.com/kerolasa/lelux-utiliteetit/commit/419a5cb277068eea395fdbdf41d9f8cad9296357

branch name: script

> +       fseek(tfile, 0, SEEK_SET);
> +       if ((size_t)st.st_size <= total)
> +               return 0;
> +       return 1;
> +}
> +
>  static double
>  getnum(const char *s)
>  {

-- 
Sami Kerola
http://www.iki.fi/kerolasa/

Re: [PATCH] scriptreplay: determine if script --quiet was used to create typescript

[Karel Zak]

On Fri, Apr 14, 2017 at 09:39:48PM +0100, Sami Kerola wrote:
> Recent commit that removed header timestamp from typescript output when
> --quiet option is defined broke scriptreplay.  Trouble was that scriptreplay
> always skipped first line of the typescript.  But --quiet makes that line to
> be part of what must be printed by scriptreplay.

This explain why I had bad feeing from the patch ;-)

> +static int
> +sfile_has_header(FILE *sfile, FILE *tfile)
> +{
> +	double delay;
> +	size_t blk, total = 0;
> +	char nl;
> +	struct stat st;
> +
> +	if (fstat(fileno(sfile), &st) < 0)
> +		return 0;
> +	while (fscanf(tfile, "%lf %zu%c\n", &delay, &blk, &nl) == 3)
> +		total += blk;
> +	fseek(sfile, 0, SEEK_SET);
> +	fseek(tfile, 0, SEEK_SET);
> +	if ((size_t)st.st_size <= total)
> +		return 0;
> +	return 1;
> +}

Not sure about it. It seems like overkill... 

I have reverted Rui's change and fixed the man page.

It seems better to keep things simple and stupid... it means keep
typescript file completely independent on the --quit option (for the
both, start and done messages).

Note that Rui also suggested to avoid strftime() for the messages due
to problems with \n (and I have suggested to use ISO times). It's
already committed.


Anyway, I don't like officially supported file formats without any
header. IMHO it would be really nice to add to the typescript:

 ### script(1) typescript; Version: util-linux v2.30; Date: 2017-04-18 12:10:43+0200

... and maybe more information. The same for timing file.

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: [PATCH] scriptreplay: determine if script --quiet was used to create typescript

[Sami Kerola]

On 18 April 2017 at 11:23, Karel Zak <kzak@redhat.com> wrote:

> Anyway, I don't like officially supported file formats without any
> header. IMHO it would be really nice to add to the typescript:
>
>  ### script(1) typescript; Version: util-linux v2.30; Date: 2017-04-18 12:10:43+0200
>
> ... and maybe more information. The same for timing file.

I agree. What comes to header data it should include header length
in bytes. Something like

### script(1) typescript; Header Length: 104 Version: util-linux
v2.30; Date: 2017-04-18 12:10:43+0200

As long it's easy to know how long to jump rest of the format does
not really matter. Right.

-- 
Sami Kerola
http://www.iki.fi/kerolasa/

Re: [PATCH] scriptreplay: determine if script --quiet was used to create typescript

[renyuneyun]

[-- Attachment #1: Type: text/plain, Size: 2009 bytes --]

On Fri, 14 Apr 2017 21:39:48 +0100, Sami Kerola wrote:
> Recent commit that removed header timestamp from typescript output when
> --quiet option is defined broke scriptreplay.  Trouble was that scriptreplay
> always skipped first line of the typescript.  But --quiet makes that line to
> be part of what must be printed by scriptreplay.
Whoops, it's the first time I use script so I wasn't aware that would be 
related to scriptreplay. Sorry for the unexpected influence ;)

Are there any special bytes or format to be used to determine that? For 
example, can we assume the first line after the header is always a 
command or empty line (meaning that nobody will try to mimic the header 
in his input)?

On Tue, 18 Apr 2017 12:23:05 +0200, Karel Zak wrote:
> Not sure about it. It seems like overkill...
>
> I have reverted Rui's change and fixed the man page.
>
> It seems better to keep things simple and stupid... it means keep
> typescript file completely independent on the --quit option (for the
> both, start and done messages).
As it comes to man page, what do you think of adding a note there to 
show why the start message isn't suppressed?

On Wed, 19 Apr 2017 19:11:57 +0100, Sami Kerola wrote:
> On 18 April 2017 at 11:23, Karel Zak <kzak@redhat.com> wrote:
>
>> Anyway, I don't like officially supported file formats without any
>> header. IMHO it would be really nice to add to the typescript:
>>
>>   ### script(1) typescript; Version: util-linux v2.30; Date: 2017-04-18 12:10:43+0200
>>
>> ... and maybe more information. The same for timing file.
> I agree. What comes to header data it should include header length
> in bytes. Something like
>
> ### script(1) typescript; Header Length: 104 Version: util-linux
> v2.30; Date: 2017-04-18 12:10:43+0200
>
> As long it's easy to know how long to jump rest of the format does
> not really matter. Right.
Forgive my witless, what's the point of introducing a length? Doesn't we 
already know the header takes (and only takes) the first line?


[-- Attachment #2: Type: text/html, Size: 2975 bytes --]

Re: [PATCH] scriptreplay: determine if script --quiet was used to create typescript

[renyuneyun]

On Fri, 14 Apr 2017 21:39:48 +0100, Sami Kerola wrote:
> Recent commit that removed header timestamp from typescript output when
> --quiet option is defined broke scriptreplay.  Trouble was that scriptreplay
> always skipped first line of the typescript.  But --quiet makes that line to
> be part of what must be printed by scriptreplay.
Whoops, it's the first time I use script so I wasn't aware that would be 
related to scriptreplay. Sorry for the unexpected influence ;)

Are there any special bytes or format to be used to determine that? For 
example, can we assume the first line after the header is always a 
command or empty line (meaning that nobody will try to mimic the header 
in his input)?

On Tue, 18 Apr 2017 12:23:05 +0200, Karel Zak wrote:
> Not sure about it. It seems like overkill...
>
> I have reverted Rui's change and fixed the man page.
>
> It seems better to keep things simple and stupid... it means keep
> typescript file completely independent on the --quit option (for the
> both, start and done messages).
As it comes to man page, what do you think of adding a note there to 
show why the start message isn't suppressed?

On Wed, 19 Apr 2017 19:11:57 +0100, Sami Kerola wrote:
> On 18 April 2017 at 11:23, Karel Zak<kzak@redhat.com>  wrote:
>
>> Anyway, I don't like officially supported file formats without any
>> header. IMHO it would be really nice to add to the typescript:
>>
>>   ### script(1) typescript; Version: util-linux v2.30; Date: 2017-04-18 12:10:43+0200
>>
>> ... and maybe more information. The same for timing file.
> I agree. What comes to header data it should include header length
> in bytes. Something like
>
> ### script(1) typescript; Header Length: 104 Version: util-linux
> v2.30; Date: 2017-04-18 12:10:43+0200
>
> As long it's easy to know how long to jump rest of the format does
> not really matter. Right.
Forgive my witless, what's the point of introducing a length? Doesn't we 
already know the header takes (and only takes) the first line?

Re: [PATCH] scriptreplay: determine if script --quiet was used to create typescript

[Sami Kerola]

On 19 April 2017 at 23:04, renyuneyun <renyuneyun@gmail.com> wrote:
>> I agree. What comes to header data it should include header length
>> in bytes. Something like
>>
>> ### script(1) typescript; Header Length: 104 Version: util-linux
>> v2.30; Date: 2017-04-18 12:10:43+0200
>>
>> As long it's easy to know how long to jump rest of the format does
>> not really matter. Right.
>
> Forgive my witless, what's the point of introducing a length? Doesn't we
> already know the header takes (and only takes) the first line?

That's true until something happens and multiline header is needed.
Secondly having a header lengh makes jumping over it really, really easy
and universal[1].  By that I mean no matter how many lines, no translator
can add new lines if they wish, scales well after header is thousands items
long, perhaps somethine else as well.  Sure this extra element header a
little longer, but I cannot think of any other downside.

[1] ref:
    long offset;
    fscanf(tfile, "### script(1) typescript; Header Length: %ld ", &offset);
    fseek(tfile, offset, SEEK_SET);

-- 
Sami Kerola
http://www.iki.fi/kerolasa/