script: handle terminal resize on SIGSWINCH

[kevans]

Summary

This is split into two commits:

commit 8ecbf42730cae3b6b50d36f453478a1afdf0780c (HEAD -> kbsd/script)
Author: Kyle Evans <[e-mail]>
Date:   Fri Apr 26 11:12:00 2024 -0500

    script: handle terminal resize on SIGWINCH
    
    Add a -w flag to forward terminal resize events on to the child, which
    can be useful in some circumstances to avoid terminal corruption.
    
    Co-authored-by: Xavier Beaudouin <[e-mail]>
    Sponsored by: Modirum MDPay
    Sponsored by: Klara, Inc.

commit 8e5a3c3e7b70eceb39c19655f8e2d75748ef7524
Author: Xavier Beaudouin <[e-mail]>
Date:   Fri Apr 26 11:10:15 2024 -0500

    script: minor style improvements
    
    Fix some nits pointed out by checkstyle9.pl in advance of functional
    changes to script(1).

[kevans]

usr.bin/script/script.c

80

Prevailing style in this file seems to be to dropping the explicit initialization, it'll be in .bss anyways

98

Consistency again- drop the name of the parameter

116

This should probably go up with win and whatnot, to maintain some semblance of the current size-descending -ish ordering

323

I'm wondering if there's a way for this to fail; might be better to error-check it, just to avoid TIOCWINSZ with a potentially bogus win? It may not hae actually been initialized earlier if stdin isn't a tty, though I wonder now if you can hit that scenario *and* get a SIGWINCH somewhere in there.

Not suggesting doing anything with the error, just only doing the TIOCSWINSZ call if this one returned != -1

[xavier.beaudouin_klarasystems.com]

Do initialization of variables in main().
Renamed the fuction onsigwinch() into resizeit().
Check if return -1 before doing the next ioctl.

[kevans]

usr.bin/script/script.c

323

[xavier.beaudouin_klarasystems.com]

Fix logic

[des]

usr.bin/script/script.c

98

You marked this as done but didn't actually do it.

113

Should be wflg for consistency.

117–118

174

279

325

This needs to be unconditional, otherwise you'll keep trying (and presumably failing) every time select() fires.

[xavier.beaudouin_klarasystems.com]

Variable names consitency.
Move do_resize=0 out of the if to avoid trying resize. May fail one time instead of always.

[xavier.beaudouin_klarasystems.com]

usr.bin/script/script.c

98

Right... Sorry

[xavier.beaudouin_klarasystems.com]

Fix commit message

[imp]

usr.bin/script/script.c

123

These still needed? The comment is lame anyway... no version, no reason, no deeper explanation.... I know you're just fixing the style, but did you try it w/o the init? It's likely for gcc 4 at this point.

302

	doresize) right?

Or do window changes not fire select?

322

is a 1 second latency sufficient? See above...

[kevans]

usr.bin/script/script.c

302

I don't believe they do; in the common case you're probably inside select(2) and you get smacked with an EINTR after returning from the signal handler.

You could perhaps block SIGWINCH and then switch to pselect(2) to unblock it and reliably interrupt it.

[xavier.beaudouin_klarasystems.com]

usr.bin/script/script.c

123

I don't know, is there any architecture supported by FreeBSD that use gcc for building ? (sorry for the stupid question indeed).

[des]

usr.bin/script/script.c

123

Yes, but I would just remove the comment as there's no harm in initializing those variables even with clang (if the compiler can tell the initialization isn't needed, it will simply not generate code for it)

[kevans]

Do signal handling to avoid races, other small fixes (-w to manpage, drop in some assertionsabout fm_fd to make sure it's set / unset when we expect it to be)

[des]

usr.bin/script/script.c

143

This should be a single-line comment between the case and the break

259

I would merge those into a single assert()

281

call me old-fashioned but I'd prefer to see these at the top

323

braces are redundant

[kevans]

Address review feedback