Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Str array converter fix#451

Merged
mattyclarkson merged 2 commits intonodegit:masternodegit/nodegit:masterfrom
mattyclarkson:str-array-converter-fixmattyclarkson/nodegit:str-array-converter-fixCopy head branch name to clipboard
Mar 2, 2015
Merged

Str array converter fix#451
mattyclarkson merged 2 commits intonodegit:masternodegit/nodegit:masterfrom
mattyclarkson:str-array-converter-fixmattyclarkson/nodegit:str-array-converter-fixCopy head branch name to clipboard

Conversation

@mattyclarkson
Copy link
Collaborator

This fixes up the conversion of string arrays. The problem is that the ConvertArray function was setting the string pointer to the internals of NanUtf8String but the class was being destroyed freeing the memory.

I also removed the need to call ConstructStrArray as that just does more string duplications (strdup) that aren't really needed.

The commits on this branch provide a clean transistion between the two methods that are more obvious that the complete branch diff.

@johnhaley81 johnhaley81 added this to the 0.3.0 milestone Mar 2, 2015
As the class is scoped to the for loop it is destroyed which means that
the memory for the string is cleared. This disallows any strings to be
correctly converted into a git_strarray.
@mattyclarkson mattyclarkson force-pushed the str-array-converter-fix branch from a54a1d0 to c843627 Compare March 2, 2015 17:45
mattyclarkson added a commit that referenced this pull request Mar 2, 2015
@mattyclarkson mattyclarkson merged commit b080622 into nodegit:master Mar 2, 2015
@mattyclarkson mattyclarkson deleted the str-array-converter-fix branch March 2, 2015 18:02
@tbranyen tbranyen modified the milestone: 0.3.0 Mar 2, 2015
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

Morty Proxy This is a proxified and sanitized view of the page, visit original site.