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

Devel #29

Merged
merged 3 commits into from
Mar 1, 2016
Merged

Devel #29

merged 3 commits into from
Mar 1, 2016

Conversation

mislavn
Copy link

@mislavn mislavn commented Feb 29, 2016

add swig javascript bindings to libyang.

Mislav Novakovic added 2 commits February 29, 2016 11:42
@codecov-io
Copy link

Current coverage is 36.05%

Merging #29 into devel will increase coverage by +0.02% as of ab56192

@@            devel     #29   diff @@
=====================================
  Files          27      27       
  Stmts       16475   16467     -8
  Branches        0       0       
  Methods         0       0       
=====================================
  Hit          5937    5937       
  Partial         0       0       
+ Missed      10538   10530     -8

Review entire Coverage Diff as of ab56192

Powered by Codecov. Updated on successful CI builds.

@rkrejci
Copy link
Collaborator

rkrejci commented Mar 1, 2016

Hi, I have 2 notes:

  1. please add some short note about building SWIG bindings into the README.md (probably as a new section after Tests) and maybe some more detailed README into the swig/ directory.

  2. building the binding generates a lot of warnings:

libyang/src/libyang.h:731: Warning 312: Nested union not currently supported (ignored).
libyang/src/tree_schema.h:443: Warning 312: Nested union not currently supported (ignored).
libyang/src/tree_schema.h:1074: Warning 312: Nested union not currently supported (ignored).
libyang/src/tree_schema.h:235: Warning 451: Setting a const char * variable may leak memory.
libyang/src/tree_schema.h:236: Warning 451: Setting a const char * variable may leak memory.
...

I'm not sure how the generated binding works, but

  • in case of Nested union it seems that the caller doesn't have access to some members that should be accessible. I'm actually fine with this warning since it seems to be a limitation of SWIG. Maybe this limitation should be better described in swig/README.
  • in case of const char* issue it seems that it generates something to set those values, but they are actually supposed to be (for the caller) read-only. Is it possible to avoid generation of such setters? Or what is the problem in this case?

Signed-off-by: Mislav Novakovic <[email protected]>
@mislavn
Copy link
Author

mislavn commented Mar 1, 2016

  1. I send you a new pull request with the two new README files. We will
    expand this with added javascript examples.

  2. For now we have used the default settings for generating javascript
    bindings. The mapping is described in swig/javascript/libyang_javascript.i.

%module libyang_javascript
%{

#include "libyang.h"
#include "tree_schema.h"
#include "tree_data.h"
#include "xml.h"
#include "dict.h"

%}

%include "libyang.h"
%include "tree_schema.h"
%include "tree_data.h"
%include "xml.h"
%include "dict.h"

Later we will modify this so the errors are fixed and the generated code is
more javascript like. For an example we will add javascripts error handling.

We will also add tests in javascript and demonstrate how to use the
javascript api. For now i will add new cmocka unit tests and when I am
finished
with that it will be easier for me to modify the javascript bindings and
add unit tests for javascript.

Best regards,
Mislav Novakovic

On 1 March 2016 at 09:55, Radek Krejčí [email protected] wrote:

Hi, I have 2 notes:

  1. please add some short note about building SWIG bindings into the
    README.md (probably as a new section after Tests) and maybe some more
    detailed README into the swig/ directory.

  2. building the binding generates a lot of warnings:

libyang/src/libyang.h:731: Warning 312: Nested union not currently supported (ignored).
libyang/src/tree_schema.h:443: Warning 312: Nested union not currently supported (ignored).
libyang/src/tree_schema.h:1074: Warning 312: Nested union not currently supported (ignored).
libyang/src/tree_schema.h:235: Warning 451: Setting a const char * variable may leak memory.
libyang/src/tree_schema.h:236: Warning 451: Setting a const char * variable may leak memory.
...

I'm not sure how the generated binding works, but

  • in case of Nested union it seems that the caller doesn't have access
    to some members that should be accessible. I'm actually fine with this
    warning since it seems to be a limitation of SWIG. Maybe this limitation
    should be better described in swig/README.
  • in case of const char* issue it seems that it generates something to
    set those values, but they are actually supposed to be (for the caller)
    read-only. Is it possible to avoid generation of such setters? Or what is
    the problem in this case?


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

@rkrejci rkrejci merged commit d8e8686 into CESNET:devel Mar 1, 2016
alangefe pushed a commit to ADTRAN/libyang that referenced this pull request Aug 7, 2018
server BUGFIX incorrectly reused variable, memory leak
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants