Re: 01/02: FFI: Add support for functions that set 'errno'.

[Mark H Weaver]

Hi Andy,

Andy Wingo <address@hidden> writes:

> wingo pushed a commit to branch master
> in repository guile.
>
> commit a396e14cb139eba37eeeea44e745bfc57bd1f37d
> Author: Mark H Weaver <address@hidden>
> Date:   Tue Jan 5 16:30:41 2016 -0500
>
>     FFI: Add support for functions that set 'errno'.
>     
>     Implements wishlist item <https://debbugs.gnu.org/18592>.
>     Requested by Frank Terbeck <address@hidden>.
>     Based on a proposed patch by Nala Ginrut <address@hidden>.
>     Patch ported to 2.2 by Andy Wingo <address@hidden>.
>     
>     * libguile/foreign.c (cif_to_procedure): Add 'with_errno' argument.
>       If true, truncate result to only one return value.
>       (scm_i_foreign_call): Separate the arguments.  Always return errno.

If I understand correctly, this unconditionally clears 'errno' before
every FFI call.  In my original version of this patch for stable-2.0, I
deliberately avoided modifying (or even accessing) 'errno' unless the
user asked to do so.  I would prefer to keep those semantics in 2.2.

More generally, it seems to me that we should avoid mutating
user-visible state when a clueful user would not expect it.  I would
certainly find it surprising for 'errno' to be modified when calling a C
function that simply retrieves a field from a C struct, for example.

What do you think?

    Thanks,
      Mark
      

> @@ -1010,7 +1042,9 @@ scm_i_foreign_call (SCM foreign, const union 
> scm_vm_stack_element *argv)
>                             max (sizeof (void *), cif->rtype->alignment));
>  
>    /* off we go! */
> +  errno = 0;
>    ffi_call (cif, func, rvalue, args);
> +  *errno_ret = errno;
>  
>    return pack (cif->rtype, rvalue, 1);
>  }