Re: [dpdk-dev] [PATCH v3 5/8] examples/power: add json string handling

["Hunt, David" <david.hunt@intel.com>]

On 25/9/2018 3:15 PM, Burakov, Anatoly wrote:
> On 25-Sep-18 3:00 PM, Hunt, David wrote:
>>> Now you're removing those newlines you added in previous commit :)
>>>
>>
>> Fixed in previous patch in the next version.
>>
>>
>>>>       if (pol->pkt.core_type == CORE_TYPE_VIRTUAL) {
>>>>           /*
>>>>            * If the cores in the policy are virtual, we need to map 
>>>> them
>>>> @@ -295,8 +423,6 @@ apply_traffic_profile(struct policy *pol)
>>>>         diff = get_pkt_diff(pol);
>>>>   -    RTE_LOG(INFO, CHANNEL_MONITOR, "Applying traffic profile\n");
>>>> -
>>>
>>> Here and in a few other places: these log message removals look to 
>>> be unrelated to this commit. Also, in my experience, more logging is 
>>> better than less logging, especially when something goes wrong - 
>>> maybe instead of removing them, just switch the level to debug?
>>>
>>
>> Changed to Debug instead. Was causing quite a verbose output.
>
> Hopefully in a separate commit :)
>

Of Course. :)

>>
>>>>       if (diff >= (pol->pkt.traffic_policy.max_max_packet_thresh)) {
>>>>           for (count = 0; count < pol->pkt.num_vcpu; count++) {
>>>>               if (pol->core_share[count].status != 1)
>>>> @@ -340,9 +466,6 @@ apply_time_profile(struct policy *pol)
>>>>                   if (pol->core_share[count].status != 1) {
>>>>                       power_manager_scale_core_max(
>>>>                           pol->core_share[count].pcpu);
>>>> -                RTE_LOG(INFO, CHANNEL_MONITOR,
>>>
>>> <snip>
>>>
>>>> +        int idx = 0;
>>>> +        int indent = 0;
>>>> +        do {
>>>> +            n_bytes = read(chan_info->fd, &json_data[idx], 1);
>>>> +            if (n_bytes == 0)
>>>> +                break;
>>>> +            if (json_data[idx] == '{')
>>>> +                indent++;
>>>> +            if (json_data[idx] == '}')
>>>> +                indent--;
>>>
>>> What happens if someone sends a string with a "{" or "}" inside?
>>>
>>
>> If we get to the end of the buffer without a "}", it calls the 
>> library to convert, will fail, and move on.  No damage done (I hope).
>> Also, a short un-terminated (by "}") string will also exit when no 
>> characters read.
>> So any invalid JSON string that's send to Jansson will fail to parse, 
>> and the application will be ready for the next (hopefully valid) JSON 
>> string.
>
> No, what i meant is something like this:
>
> { "json_value": "{"}
>
> According to JSON validator, this is a valid JSON string, but it will 
> break your code :)
>

You are correct if this code was designed to be a general purpose JSON 
string reader. However, it's only designed to take in strings for this 
sample application, and they do not expect any brace characters embedded 
within quotes. So I think it's OK for this use case. Patches welcome, 
though! :)

Thanks,
Dave.