[DTrace-devel] [PATCH] Add support for built-in variable errno

[Kris Van Hees]

Reviewed-by: Kris Van Hees <kris.van.hees at oracle.com>

... with the changes listed below.

On Mon, Apr 26, 2021 at 01:46:29PM -0400, Eugene Loh wrote:
> On 4/24/21 9:57 PM, Kris Van Hees wrote:
> 
> > On Fri, Apr 23, 2021 at 12:50:12PM -0400, eugene.loh at oracle.com wrote:
> >> From: Eugene Loh <eugene.loh at oracle.com>
> >>
> >> The built-in variable errno should be:
> >>
> >>    *)  the errno for failed system calls for syscall:::return probes
> >>
> >>    *)  0 otherwise
> >>
> >> This behavior is slightly different from the errno in C library functions
> >> (where errno is not reset by successful system calls) and from the
> >> behavior described in the DTrace Guide (which allows errno to be non-0
> >> for non-syscall probes).
> > The behaviour of errno in the C library is not relevant here because this is
> > the D variable 'errno' which has no relation with the C library variable of the
> > same name.
> 
> It seems to me that the two definitely have similarities. Meanwhile, the 
> text is trying to clarify that the two are different.  It seems to me 
> that far, far more users are familiar with libc's errno than are 
> familiar with the semantic differences between it and DTrace's.  I vote 
> for keeping the sentence, but if you want then let me know and I'll 
> strike it.

I would just simply state that the D errno variable is different from the C
library errno variable.

> > As far as the DTrace guide goes... the description there is not
> > as accurate as I would like because the reference implementation (DTrace on
> > Oracle Solaris) is very OS specific and Linux does not have any mechanism to
> > provide the exact same functionality.
> 
> Yeah.  Anyhow, that's the situation.  So, thanks for the clarification, 
> but I assume there are no implications for this patch?

Correct.

> >> Introduce an mst->syserrno, and have the provider trampolines set it to 0.
> > Let's name it mst->errno for consistency with the errno variable itself.
> Yeah.
> 
> Unfortunately, the naming collides with errno.h, which defines
> 
>          # define errno (*__errno_location ())
> 
> and makes using the string "errno" tricky.  E.g.,
> 
>      $ cat play1.c
>      typedef struct dt_mstate {
>          int errno;
>      } dt_mstate_t;
>      $ gcc -c play1.c
> 
> is okay.  Contrast that to when we add "#include <errno.h>":
> 
>      $ cat play2.c
>      #include <errno.h>
>      typedef struct dt_mstate {
>          int errno;
>      } dt_mstate_t;
>      $ gcc -c play2.c
>      In file included from play2.c:1:
>      play2.c:3:6: error: field ???__errno_location??? declared as a function
>        int errno;
>            ^~~~~
> 
> And some files (like dt_cg.c, among others) want to include both errno.h 
> and dt_dctx.h.  (So, once again, DTrace can like using the name "errno" 
> but the 900-pound gorilla in the room has already claimed it.)

Then may I suggest using 'sc_errno', to make it more clear that it is the
syscall (sc) errno, and not some other system errno?