From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <anatoly.burakov@intel.com>
Received: from mga07.intel.com (mga07.intel.com [134.134.136.100])
 by dpdk.org (Postfix) with ESMTP id 3F60C1B108
 for <dev@dpdk.org>; Tue, 25 Sep 2018 16:15:46 +0200 (CEST)
X-Amp-Result: SKIPPED(no attachment in message)
X-Amp-File-Uploaded: False
Received: from fmsmga005.fm.intel.com ([10.253.24.32])
 by orsmga105.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384;
 25 Sep 2018 07:15:45 -0700
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.54,302,1534834800"; d="scan'208";a="265571763"
Received: from aburakov-mobl1.ger.corp.intel.com (HELO [10.237.220.55])
 ([10.237.220.55])
 by fmsmga005.fm.intel.com with ESMTP; 25 Sep 2018 07:15:43 -0700
To: "Hunt, David" <david.hunt@intel.com>, dev@dpdk.org
Cc: john.mcnamara@intel.com, stephen@networkplumber.org, lei.a.yao@intel.com, 
 bruce.richardson@intel.com
References: <20180912144930.50578-1-david.hunt@intel.com>
 <20180914135406.52190-1-david.hunt@intel.com>
 <20180914135406.52190-6-david.hunt@intel.com>
 <47b4dbc6-48ca-f41c-86cb-e6f5b168ebf9@intel.com>
 <046d9290-55f9-8433-cbbd-b975f353fa47@intel.com>
From: "Burakov, Anatoly" <anatoly.burakov@intel.com>
Message-ID: <5c1ca576-95ff-e150-7c4e-47bf4bfc255b@intel.com>
Date: Tue, 25 Sep 2018 15:15:42 +0100
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101
 Thunderbird/52.9.1
MIME-Version: 1.0
In-Reply-To: <046d9290-55f9-8433-cbbd-b975f353fa47@intel.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Subject: Re: [dpdk-dev] [PATCH v3 5/8] examples/power: add json string
 handling
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Tue, 25 Sep 2018 14:15:46 -0000

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 :)

> 
>>>       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 :)

-- 
Thanks,
Anatoly