Automated Go bindings & Errors

[shermp]

Hi @NiLuJe

I've started work on automated bindings using c-for-go here: https://github.com/shermp/FBInk/tree/go-bindings

I'm at the point where I seem to have a working binary, at least with the very limited testing I've done with it.

What I haven't decided on is how I'm actually going to deal with errors. Currently, the functions just return ints with no context.

I hate to say it, but dealing with C errno's is a bit of a PITA, doubly so when they're negative. To get the errno constants in c-for-go, one would have to find and include the appropriate errno.h file, which is compiler specific etc.

Have you any ideas on how I might be able to deal with FBInk errors, hopefully without too mich manual coding?

[pgaskin]

I did notice that almost every error has a WARN with a relevant message before it, so maybe that could be modified to save the last warning into a global thread-local variable accessed by the Go wrapper, or something of that sort?

[shermp]

Hmm, actually, the approach I took with my handwritten bindings should do the trick:

package fbink

/*
#cgo LDFLAGS: -L${SRCDIR}/../../Release -lfbink -lm
#include <stdlib.h>
#include <errno.h>
*/
import "C"

// Error codes as returned by FBInk
const (
	ExitSuccess int32 = int32(C.EXIT_SUCCESS)
	ExitFailure int32 = int32(C.EXIT_FAILURE) * -1
	ENoDev      int32 = int32(C.ENODEV) * -1
	ENotSup     int32 = int32(C.ENOTSUP) * -1
	ENoData     int32 = int32(C.ENODATA) * -1
	ETime       int32 = int32(C.ETIME) * -1
	EInval      int32 = int32(C.EINVAL) * -1
	EIlSeq      int32 = int32(C.EILSEQ) * -1
	ENoSpc      int32 = int32(C.ENOSPC) * -1
)

There should be minimal, if any error changes between FBInk releases, so this shouldn't be an issue.

EDIT: This is going to be a very low level wrapper, so I'm not going to worry about wrapping errors in an actual error type or anything this time around.

[pgaskin]

EDIT: This is going to be a very low level wrapper, so I'm not going to worry about wrapping errors in an actual error type or anything this time around.

Oh, I see what you're getting at. I was thinking you were wanting full error messages and such.

ExitSuccess int32 = int32(C.EXIT_SUCCESS)

Yes, that should work. That's the approach I usually take when doing that kind of thing (but I make it a type and add an Error method to it for a nicer message).

[shermp]

Yes, that should work. That's the approach I usually take when doing that kind of thing (but I make it a type and add an Error method to it for a nicer message).

Yeah, I would (and have) done that when hand-writing a wrapper. c-for-go is spitting out int32 return types though, so that's what I'm using here.

Also, FBInk error numbers are often context sensitive. The same error number might have different meanings depending on the function that returned it. That means I can't really have a generic message for each error number.

Basically, the users will need to consult fbink.h to deal with errors correctly.

[NiLuJe]

I was out all day, glad you figured it out ;p.

The actual error constants should be pretty much set in stone, as most of them are either mandated by POSIX, or Linux, and since this isn't ever going to run on anything other than Linux for obvious reasons, <errno.h> should cover it pretty much everywhere (i.e., you can rely on the host's in standard paths if getting the paths right for a cross toolchain was the pain point) ;).

[NiLuJe]

On a related note, I'm not quite sure how I feel about having auto-generated binding code in the main repo.

I'm perfectly fine with the specific config(s) needed for the tool (i.e., the YAML config here), much like how the cdecl stuff is kept here for the Lua/Python bindings, but I kinda feel like the generated code ought to stay in a dedicated repo?

(Not familiar with the Go ecosystem, so, open to any kind of comments on the subject ;)).

[shermp]

I was wondering about that myself to be honest, Part of the problem is that the module version of go get does not fetch submodules. On the other hand. I might advise against go get, because one has to build FBInk anyway.

Part of the appeal of having the bindings in the main repo, is one can do the following:

# git clone fbink
make pic
cd gofbink/fbink
go build

It would mean users wouldn't have to install c-for-go themselves to use the library.

My view is that the configuration controlling the code generation should live in the same repo as the generated code itself.

If you wish though, I can separate everything out into it's own repo, but that will require more maintenance to keep everything up to date.

Hmm. decisions decisions...

[NiLuJe]

@shermp: Those are pretty valid concerns, and I definitely think the deciding factor should be ease of maintenance, so, if that helps, go (ha!) for it ;).

[shermp]

Maintenance is why I'm investigating auto generated bindings in the first place :p

Still not entirely sold on having it all in the main FBInk repo though (thinking about go mod stuff), so I won't completely commit to it just yet.

Unfortunately, the Go philosophy is that pre Go modules, master never introduces breaking API changes, and post Go modules, minor/patch versions never break the API, therefore pinning to a specific git commit is unnecessary, therefore git submodules are unnecessary.

Which is fine if one is using pure Go libraries. A bit more awkward with C libraries like FBInk.

[NiLuJe]

Random unrelated comments, in no particular order:

• -fshort-enums can be used to make GCC use the smallest wide-enough integer type for an enum (instead of int). With the obvious caveat that it essentially breaks ABI by doing that.
  Here, that'd only really be a problem for fbink_add_ot_font, as that's the only function that takes an enum typedef directly instead of a basic C type (and that's mostly my bad, I should have caught it on review, as I'm pretty sure I'd already been using enum values without going through their enum typedef by then).

EDIT: Speaking of enums, not quite sure how problematic it might be if c-for-go honors the compiler's bitness when doing the conversion (i.e., they're int, meaning int32_t on ARM, but int64_t on x86_64).
I'm mentioning this because it happens to be an issue with size_t with ffi-cdecl: it tends to represent that data type in its basic form for the compiler's bitness (f.g., long unsigned int instead of size_t), which wreaks havoc if you keep it that way, and then run the generated code at a different bitness...

• GCC can print its sysroot and search paths via -print-sysroot and -print-search-dirs.