Skip to content
This repository has been archived by the owner on Jun 7, 2021. It is now read-only.

[TRAFODION-2772] - retrieve a value from Json string got an error #1439

Merged
merged 3 commits into from Feb 7, 2018

Conversation

andyyangcn
Copy link
Contributor

No description provided.

@Traf-Jenkins
Copy link

Check Test Started: https://jenkins.esgyn.com/job/Check-PR-master/2416/

@Traf-Jenkins
Copy link

Copy link
Contributor

@DaveBirdsall DaveBirdsall left a comment

Choose a reason for hiding this comment

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

Please consider using stack variables instead of heap variables.

char *rltStr = NULL;
JsonReturnType ret = json_extract_path_text(&rltStr, op_data[1], 1, op_data[2]);
char *jsonStr = new(heap) char[len1+1];
char *jsonAttr = new(heap) char[len2+1];
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm wondering if we need to delete jsonStr and jsonAttr after the json_extract_path_text call to avoid unnecessary heap pressure. Though if json_extract_path_text itself does new's on the same heap, we'd get heap fragmentation.

Another approach would be to allocate these on the stack instead, avoiding both concerns:

char jsonStr[len1+1];
char jsonAttr[len2+1];
...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since the value of len1 can only be determined when executing the query, we cannot define jsonStr as an Array.
Will release the memory manually.

Copy link
Contributor

Choose a reason for hiding this comment

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

But we can! C++ allows dynamically-sized arrays on the stack. Try it!

Copy link
Contributor

Choose a reason for hiding this comment

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

Here's an example. Compile this program:

// Demonstrate dynamically-sized arrays on C++ stack

#include <iostream>

using namespace std;

int main(int argc, char * argv[])
{
for (size_t i = 5; i < 10; i++)
  {
    char temp[i+1];
    cout << "sizeof(temp) = " << sizeof(temp) << endl;
  }

return 0;
}

When you run it, you'll get this output:

[birdsall@edev08 dynChar]$ ./dynChar.exe
sizeof(temp) = 6
sizeof(temp) = 7
sizeof(temp) = 8
sizeof(temp) = 9
sizeof(temp) = 10
[birdsall@edev08 dynChar]$

@Traf-Jenkins
Copy link

New Check Test Started: https://jenkins.esgyn.com/job/Check-PR-master/2421/

@Traf-Jenkins
Copy link

@Traf-Jenkins
Copy link

New Check Test Started: https://jenkins.esgyn.com/job/Check-PR-master/2425/

@Traf-Jenkins
Copy link

Copy link
Contributor

@DaveBirdsall DaveBirdsall left a comment

Choose a reason for hiding this comment

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

+1 Looks good. Thanks for considering my suggestions.

@asfgit asfgit merged commit f8726b4 into apache:master Feb 7, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
4 participants