Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add C4Listener and JNI support #6

Merged
merged 2 commits into from
Jun 9, 2020
Merged

Add C4Listener and JNI support #6

merged 2 commits into from
Jun 9, 2020

Conversation

bmeike
Copy link
Contributor

@bmeike bmeike commented Jun 9, 2020

Sketch in C4Listener and its JNI support.

//
// native_c4listener.cc
//
// Copyright (c) 2017 Couchbase, Inc All rights reserved.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// Copyright (c) 2017 Couchbase, Inc All rights reserved.
// Copyright (c) 2020 Couchbase, Inc All rights reserved.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

Copy link
Contributor

@pasin pasin left a comment

Choose a reason for hiding this comment

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

Have a few minor comments.

FleeceStatic
"-Wl,--no-whole-archive"
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this intention (-Wl,--whole-archive and -Wl,--no-whole-archive)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. It doesn't work wo/ that. Seems like a linker bug to me. Jim says it is SOP.

config.allowCreateDBs = allowCreateDBs;
config.allowDeleteDBs = allowDeleteDBs;

auto listener = c4listener_start(&config, &error);
Copy link
Contributor

Choose a reason for hiding this comment

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

Throws if error occurs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. Done.

Java_com_couchbase_lite_internal_core_C4Listener_getUriFromPath
(JNIEnv *env, jclass clazz, jlong c4Listener, jstring path) {
jstringSlice pathSlice(env, path);
auto uri = c4db_URINameFromPath(pathSlice);
Copy link
Contributor

Choose a reason for hiding this comment

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

IIRC, we may not need this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is in the spec!!!


static {
final Map<KeyMode, Integer> m = new HashMap<>();
m.put(KeyMode.CERT, 1);
Copy link
Contributor

Choose a reason for hiding this comment

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

For consistency, use hex like OPTION_TO_C4 or the other way around?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh... ok. Sure.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wait. Options are bit flags. They need to be hex. Certs modes are not. They are mutually exclusive.

These declarations are identical to the ones in c4Listener.cc

@bmeike bmeike merged commit 8828b00 into master Jun 9, 2020
@bmeike bmeike deleted the p2p/c4listener branch June 9, 2020 23:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants