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

cleanup strdup nonsense using new opt pattern #179

Closed
defunctzombie opened this issue Jan 6, 2013 · 4 comments
Closed

cleanup strdup nonsense using new opt pattern #179

defunctzombie opened this issue Jan 6, 2013 · 4 comments

Comments

@defunctzombie
Copy link
Collaborator

Avoid strdup where possible by just capturing the Handle value. We cannot simple capture the Utf8Value as there is no empty constructor.

Example of capturing handles and getting strings or NULL.

v8::Handle<v8::Value> extIdOpt;
v8::Handle<v8::Value> sysIdOpt;

//must be set to null in order for xmlCreateIntSubset to ignore them
if (args.Length() > 1 && args[1]->IsString()) {
    extIdOpt = args[1];
}
if (args.Length() > 2 && args[2]->IsString()) {
    sysIdOpt = args[2];
}

v8::String::Utf8Value extIdRaw(extIdOpt);
v8::String::Utf8Value sysIdRaw(sysIdOpt);

const char* extId = (extIdRaw.length()) ? *extIdRaw : NULL;
const char* sysId = (sysIdRaw.length()) ? *sysIdRaw : NULL;
ncb000gt added a commit that referenced this issue Jan 6, 2013
Remove the use of strdup and associated frees and use the opt pattern.

Signed-off-by: Nick Campbell <nicholas.j.campbell@gmail.com>
@ncb000gt
Copy link
Collaborator

ncb000gt commented Jan 6, 2013

The only other place I found reference to strdup was in libxmljs.cc:80 using xmlMemoryStrdup. This isn't going to be replaced this way, and we probably don't want to change this as it is likely fundamental to libxml. :)

Anyway, the above commit should cover it.

@ncb000gt ncb000gt closed this as completed Jan 6, 2013
@defunctzombie
Copy link
Collaborator Author

LGTM. If all tests pass merge it in!

@ncb000gt
Copy link
Collaborator

ncb000gt commented Jan 6, 2013

I've run the tests and while they fail, they are failing with a segfault in the ref_integrity tests. I took out the failing test and all is well so the code itself is good. I'll leave it in master (I'm not going to take out the failing test).

@defunctzombie
Copy link
Collaborator Author

Sounds good
On Jan 6, 2013 1:50 PM, "Nick Campbell" notifications@github.com wrote:

I've run the tests and while they fail, they are failing with a segfault
in the ref_integrity tests. I took out the failing test and all is well so
the code itself is good. I'll leave it in master (I'm not going to take out
the failing test).


Reply to this email directly or view it on GitHubhttps://github.com//issues/179#issuecomment-11932313.

skabbes pushed a commit to CodeFridge/libxmljs that referenced this issue Aug 15, 2013
Remove the use of strdup and associated frees and use the opt pattern.

Signed-off-by: Nick Campbell <nicholas.j.campbell@gmail.com>
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

No branches or pull requests

2 participants