Thanks for the feedback Maxime, I obviously lost track of some things after letting this patch languish for a while.
Maxime Devos <maximede...@telenet.be> writes:
> On 17-09-2022 10:05, Christopher Baines wrote:
>> + if (ioptname == SO_RCVTIMEO || ioptname == SO_SNDTIMEO)
>> + {
>> + SCM_ASSERT (scm_is_pair (value), value, SCM_ARG4, FUNC_NAME);
>
> How about using SCM_ASSERT_TYPE instead to improve the error message a
> little?
I've switched to using SCM_ASSERT_TYPE now.
>> + opt_time.tv_sec = scm_to_ulong (SCM_CAR (value));
>> + opt_time.tv_usec = scm_to_ulong (SCM_CDR (value));
> I think it needs to be documented how to actually use SO_RCVTIMEO /
> SO_SNDTIMEO (more specifically, the types). For example, the user
> might expect to enter flonums 1.567 as time durations (in seconds)
> instead, or a duration from SRFI 19. How is the user supposed to know
> the appropriate type, aside from experimenting and reading the source
> code?
>
> Also, it is not documented there is a maximum on the number of
> seconds, I recommend to document that there is some limit, or
> alternatively clip numbers to the maximum.
I've added some documentation now, I'm not sure the interface is
perfect, but it should work.
>> + optlen = sizeof (opt_time);
>> + optval = &opt_time;
>> + }
>> +
>
> I think this needs a #if defined(SO_RCVTIMEO) && defined(SO_SNDTIMEO),
> for systems that don't have those. Alternatively, if all systems have
> those, it seems to me that the #ifdef in
I guess it should be conditional, so I've added the if in to getsockopt
and setsockopt.
>> +#ifdef SO_RCVTIMEO
>> + scm_c_define ("SO_RCVTIMEO", scm_from_int (SO_RCVTIMEO));
>> +#endif
>> +#ifdef SO_SNDTIMEO
>> + scm_c_define ("SO_SNDTIMEO", scm_from_int (SO_SNDTIMEO));
>> +#endif
>
> can be removed.
signature.asc
Description: PGP signature
