0

I was trying to learn and understand cgo. I am writing a program in Go where I connect to AD and parse the output. I tested the code in C, which is working fine as expected. The relevant part in C is

char *ldap_host = "x.x.x.x";
int   ldap_port     = 389;
ldap = ldap_init(ldap_host, ldap_port)) 

Now I am trying to get the same working in go as

//#cgo CFLAGS: -lldap 
//#include <ldap.h>
import "C"
func main() {
    var hostname *string
    hostname = &os.Args[1]
    ldap_port := 389
    ldap := C.ldap_init(*hostname, ldap_port)
}

But I am getting the following error

could not determine kind of name for C.ldap_init

What am I doing wrong here?

JimB
  • 104,193
  • 13
  • 262
  • 255
scott
  • 1,557
  • 3
  • 15
  • 31

1 Answers1

4

I would start by getting the basics of Go and cgo working before adding the individual peculiarities of a library like ldap. Some good starting points, especially the official cgo doc page.

Starting from the top:

You don't need the CFLAGS here, but you will need LDFLAGS for the linker, and liblber for this to run.

#cgo LDFLAGS: -lldap -llber

You can't pass a pointer to a Go string as a C *char, they are different types. Strings in Go actually aren't addressable at all, so this would be a compile error if the build process got that far. Use C.CString, and C.free if you need to create C strings. You will also need to include stdlib.h for C.free.

    url := C.CString(os.Args[1])
    defer C.free(unsafe.Pointer(url))

Go's int size varies by architecture, and is not C's int type. ldap_port in your example is converted to the default type of int, where you would have needed C.int.

Finally, the original error in your question has nothing to do with Go or cgo. The ldap_init function is deprecated, and does not exist without setting LDAP_DEPRECATED. You should use the ldap_initialize function. Here is a minimal example that compiles:

package main

import (
    "log"
    "unsafe"
)

/*
#include <stdlib.h>
#include <ldap.h>

#cgo LDFLAGS: -lldap -llber
*/
import "C"

func main() {
    url := C.CString("ldap://127.0.0.1:389/")
    defer C.free(unsafe.Pointer(url))

    var ldap *C.LDAP

    rv := C.ldap_initialize(&ldap, url)
    if rv != 0 {
        log.Fatalf("ldap_initialize() error %d", rv)
    }
    log.Println("initialized")
}
JimB
  • 104,193
  • 13
  • 262
  • 255
  • Thank you @JimB. Appreciate the pointers on where to learn this. – scott Feb 08 '16 at 16:33
  • I wonder if this is 100% safe. From what I could understand in [#12416](https://github.com/golang/go/issues/12416) and [#8310](https://github.com/golang/go/issues/8310), it is incorrect to pass Go pointer to C function when you don't know if the Go pointer will be kept by C side. I don't expect ldap to do this, but would it not be more safe to C.malloc that pointer (*LDAP, not LDAP)? – tumdum Feb 09 '16 at 08:37
  • @TomaszKłak: This is fine. #8310 is irrelevant, and was superseded by #12416. The memory for `ldap` here is allocated in C when passing a `**C.LDAP`, where the pointer itself is filled in by the C code, pointing to C memory. The go1.6 cgo checks are enabled by default, and will crash the program rather than allow potentially unsafe behavior. – JimB Feb 09 '16 at 13:52
  • Oh, you did check this under 1.6. I know that memory allocated by `ldap_initialize` was not a problem here. What I was worried about is that `ldap_initialize` might store adress of Go variable `ldap` somewhere in some global C value. That would lead to C having pointer to Go value after C call ended. – tumdum Feb 09 '16 at 13:55
  • @TomaszKłak: My local go build is only a couple days old from master. Even if the value of `ldap` is stored in C, it's not a pointer to Go memory. It starts off as `nil`, and is filled in by the initialize function. So what you have is a pointer value on the Go heap, pointing to C memory, which we've deemed safe. It's just as if `ldap_initialize` returned an `*LDAP` which we assign to `ldap`, but since C doesn't have multiple return values or exceptions, it's common to pass a `**` as a parameter instead. – JimB Feb 09 '16 at 14:05