>+      // Not necessary if ttyname() must also respect TTY_NAME_MAX.
>+      // POSIX ttyname description isn't completely clear.
Not necessary -> not certain. I don’t think the ‘#define FUNC_NAME 
"scm_i_c_ttyname"’ is actually doing something for scm_i_ttyname, given the 
absence of SCM_SYSERROR. But maybe it’s a consistency thing. I don’t recall 
‘//’ comments being in use in Guile but maybe I just forgot. Since the internal 
function is only used in the same file as which it is defined in, you can make 
it a static function and remove it from posix.h.

And shouldn’t be the description of the function be right before the function 
declaration (and maybe before the #if too, dunno) instead of inside?

Also, the locking is like

lock()
do syscall and maybe raise exception (via asyncs in case of a weird EINTR(*), I 
don’t refer to system-error here)
unlock()

(*) see Hurd situation I mentioned.

You need to also ‘unlock’ in the case of an exception (maybe 
scm_i_scm_pthread_mutex_lock does so, but going by the name I wouldn’t expect 
it to.) I don’t know what the simplest way to do this, but the dynwind code 
could solve this.  Or, alternatively, block asyncs for a little while.

Simpler solution: all that SCM_SYSCALL does, if I remember correctly, is 
running the syscall in a loop and every iteration do a tick. But since EINTR is 
non-standard(IIUC), no loop is needed so we can just _not_ treat EINTR 
specially and raise an exception like in other cases (which happens outside the 
lock). And if you remove the EINTR handling, all you end up with is a simple 
syscall (and _maybe_ an async tick, which if kept could simply be moved after 
the unlock).

On length checks and the like: it isn’t clear to me whether TTY_NAME_MAX 
includes the terminating zero or not (I would expect not, like strlen). 
Although, ‘namesize’ _does_ include the terminating zero going by what’s 
written (*) (“The array is namesize characters long”, + see [ERANGE] 
description.).

(*) source: https://pubs.opengroup.org/onlinepubs/9699919799/, in particular 
ttyname.

(In case of unclarity and to be really sure to not copy too much, strncpy 
exists where n=length of array excluding space reserved for terminating zero, 
and things could be initialised to \0.)

Best regards,
Maxime Devos

Reply via email to