Skip to content
This repository has been archived by the owner on Feb 24, 2020. It is now read-only.

stage1, pkg: add acl package #1963

Merged
merged 6 commits into from Jan 29, 2016
Merged

Conversation

iaguis
Copy link
Member

@iaguis iaguis commented Jan 13, 2016

This package is a wrapper over libacl that, instead of being linked to
it, it dlopens it so we can check if it's present on the system.

It's not meant to be complete, we just implement the functions needed by
rkt at the moment.

It is based on https://github.com/naegelejd/go-acl

Fixes #1961

@iaguis
Copy link
Member Author

iaguis commented Jan 13, 2016

Waiting for an answer in naegelejd/go-acl#1

naegelejd added a commit to naegelejd/go-acl that referenced this pull request Jan 13, 2016
at the request of @iaguis for rkt/rkt#1963
closes #1
@iaguis
Copy link
Member Author

iaguis commented Jan 13, 2016

Waiting for an answer in naegelejd/go-acl#1

The author answered saying that the license will be MIT, which should be compatible with our Apache.

Also, added an acknowledgement.

EDIT: There's a LICENSE file now :)

@iaguis
Copy link
Member Author

iaguis commented Jan 26, 2016

Do we want this for 1.0?

cc @jonboulle

@jonboulle
Copy link
Contributor

I think it would be great - what's left to do?

@iaguis
Copy link
Member Author

iaguis commented Jan 26, 2016

Make sure we include the appropriate license text/files and a review.

On Tue, Jan 26, 2016, 17:38 Jonathan Boulle notifications@github.com
wrote:

I think it would be great - what's left to do?


Reply to this email directly or view it on GitHub
#1963 (comment).

@iaguis
Copy link
Member Author

iaguis commented Jan 27, 2016

Rebased and added the MIT license to LICENSE

// {
// acl_t (*acl_from_text)(const char *buf_p);
//
// acl_from_text = (acl_t (*)(const char *))f;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are the casts from a void pointer needed?

This package is a wrapper over libacl that, instead of being linked to
it, it dlopens it so we can check if it's present on the system.

It's not meant to be complete, we just implement the functions needed by
rkt at the moment.

It is based on https://github.com/naegelejd/go-acl
We're not linking to libacl.so, but we dlopen it.
sym := C.CString(symbol)
defer C.free(unsafe.Pointer(sym))

p := C.dlsym(handle, sym)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to manual, the proper way of loading symbols is to call dlerror() to clear an error message, then dlsym and then dlerror again to check if there was an error in dlsym. So I guess something like follows would cut it:

C.dlerror()
p := C.dlsym(…)
e := C.dlerror()
if e != nil {
    return nil, errwrap(fmt.Errorf("error resolving symbol %q", symbol), errors.New(C.GoString(e))
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. I guess I'll wrap all the errors then.

BTW, we should also do this in go-systemd

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That calls for a new package! :P

@iaguis
Copy link
Member Author

iaguis commented Jan 29, 2016

Updated, I think I addressed everything. PTAL

@jonboulle
Copy link
Contributor

LGTM, this is tested right? :D

@iaguis
Copy link
Member Author

iaguis commented Jan 29, 2016

LGTM, this is tested right? :D

Natürlich! It works on my machine (TM)

@krnowak
Copy link
Collaborator

krnowak commented Jan 29, 2016

Also works on my machine. LFAD.

krnowak added a commit that referenced this pull request Jan 29, 2016
@krnowak krnowak merged commit 540dd52 into rkt:master Jan 29, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants