* [PATCH 0/3] perf tools: Enable strict JSON parsing @ 2021-10-07 11:05 ` James Clark 0 siblings, 0 replies; 36+ messages in thread From: James Clark @ 2021-10-07 11:05 UTC (permalink / raw) To: acme, john.garry, ak, linux-perf-users Cc: Nick.Forrington, Andrew.Kilroy, James Clark, Will Deacon, Mathieu Poirier, Leo Yan, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim, linux-arm-kernel, linux-kernel After a discussion on "[PATCH 1/4] perf vendor events: Syntax corrections in Neoverse N1 json", John Garry suggested that we can just modify the parser to make it more strict. Hopefully this will remove the need to apply any future JSON comma fixup commits. Applies on top of "[PATCH v2 1/3] perf vendor events: Syntax corrections in Neoverse N1 json" on perf/core. Also available at: git clone --branch james-json-parse-fix git@git.gitlab.arm.com:linux-arm/linux-jc.git James Clark (3): perf vendor-events: Fix all remaining invalid JSON files perf tools: Make the JSON parser more conformant when in strict mode perf tools: Enable strict JSON parsing .../arch/arm64/ampere/emag/bus.json | 2 +- .../arch/arm64/ampere/emag/cache.json | 20 ++++----- .../arch/arm64/ampere/emag/clock.json | 2 +- .../arch/arm64/ampere/emag/exception.json | 4 +- .../arch/arm64/ampere/emag/instruction.json | 10 ++--- .../arch/arm64/ampere/emag/memory.json | 4 +- .../arch/arm64/hisilicon/hip08/metrics.json | 2 +- .../pmu-events/arch/nds32/n13/atcpmu.json | 2 +- .../pmu-events/arch/s390/cf_z10/basic.json | 2 +- .../pmu-events/arch/s390/cf_z10/crypto.json | 2 +- .../pmu-events/arch/s390/cf_z10/extended.json | 2 +- .../pmu-events/arch/s390/cf_z13/basic.json | 2 +- .../pmu-events/arch/s390/cf_z13/crypto.json | 2 +- .../pmu-events/arch/s390/cf_z13/extended.json | 2 +- .../pmu-events/arch/s390/cf_z14/basic.json | 2 +- .../pmu-events/arch/s390/cf_z14/crypto.json | 2 +- .../pmu-events/arch/s390/cf_z14/extended.json | 2 +- .../pmu-events/arch/s390/cf_z15/basic.json | 2 +- .../pmu-events/arch/s390/cf_z15/crypto.json | 2 +- .../pmu-events/arch/s390/cf_z15/crypto6.json | 2 +- .../pmu-events/arch/s390/cf_z15/extended.json | 2 +- .../pmu-events/arch/s390/cf_z196/basic.json | 2 +- .../pmu-events/arch/s390/cf_z196/crypto.json | 2 +- .../arch/s390/cf_z196/extended.json | 2 +- .../pmu-events/arch/s390/cf_zec12/basic.json | 2 +- .../pmu-events/arch/s390/cf_zec12/crypto.json | 2 +- .../arch/s390/cf_zec12/extended.json | 2 +- .../arch/test/test_soc/cpu/uncore.json | 2 +- .../arch/x86/icelakex/icx-metrics.json | 2 +- tools/perf/pmu-events/jsmn.c | 43 ++++++++++++++++++- 30 files changed, 85 insertions(+), 46 deletions(-) -- 2.28.0 ^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 0/3] perf tools: Enable strict JSON parsing @ 2021-10-07 11:05 ` James Clark 0 siblings, 0 replies; 36+ messages in thread From: James Clark @ 2021-10-07 11:05 UTC (permalink / raw) To: acme, john.garry, ak, linux-perf-users Cc: Nick.Forrington, Andrew.Kilroy, James Clark, Will Deacon, Mathieu Poirier, Leo Yan, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim, linux-arm-kernel, linux-kernel After a discussion on "[PATCH 1/4] perf vendor events: Syntax corrections in Neoverse N1 json", John Garry suggested that we can just modify the parser to make it more strict. Hopefully this will remove the need to apply any future JSON comma fixup commits. Applies on top of "[PATCH v2 1/3] perf vendor events: Syntax corrections in Neoverse N1 json" on perf/core. Also available at: git clone --branch james-json-parse-fix git@git.gitlab.arm.com:linux-arm/linux-jc.git James Clark (3): perf vendor-events: Fix all remaining invalid JSON files perf tools: Make the JSON parser more conformant when in strict mode perf tools: Enable strict JSON parsing .../arch/arm64/ampere/emag/bus.json | 2 +- .../arch/arm64/ampere/emag/cache.json | 20 ++++----- .../arch/arm64/ampere/emag/clock.json | 2 +- .../arch/arm64/ampere/emag/exception.json | 4 +- .../arch/arm64/ampere/emag/instruction.json | 10 ++--- .../arch/arm64/ampere/emag/memory.json | 4 +- .../arch/arm64/hisilicon/hip08/metrics.json | 2 +- .../pmu-events/arch/nds32/n13/atcpmu.json | 2 +- .../pmu-events/arch/s390/cf_z10/basic.json | 2 +- .../pmu-events/arch/s390/cf_z10/crypto.json | 2 +- .../pmu-events/arch/s390/cf_z10/extended.json | 2 +- .../pmu-events/arch/s390/cf_z13/basic.json | 2 +- .../pmu-events/arch/s390/cf_z13/crypto.json | 2 +- .../pmu-events/arch/s390/cf_z13/extended.json | 2 +- .../pmu-events/arch/s390/cf_z14/basic.json | 2 +- .../pmu-events/arch/s390/cf_z14/crypto.json | 2 +- .../pmu-events/arch/s390/cf_z14/extended.json | 2 +- .../pmu-events/arch/s390/cf_z15/basic.json | 2 +- .../pmu-events/arch/s390/cf_z15/crypto.json | 2 +- .../pmu-events/arch/s390/cf_z15/crypto6.json | 2 +- .../pmu-events/arch/s390/cf_z15/extended.json | 2 +- .../pmu-events/arch/s390/cf_z196/basic.json | 2 +- .../pmu-events/arch/s390/cf_z196/crypto.json | 2 +- .../arch/s390/cf_z196/extended.json | 2 +- .../pmu-events/arch/s390/cf_zec12/basic.json | 2 +- .../pmu-events/arch/s390/cf_zec12/crypto.json | 2 +- .../arch/s390/cf_zec12/extended.json | 2 +- .../arch/test/test_soc/cpu/uncore.json | 2 +- .../arch/x86/icelakex/icx-metrics.json | 2 +- tools/perf/pmu-events/jsmn.c | 43 ++++++++++++++++++- 30 files changed, 85 insertions(+), 46 deletions(-) -- 2.28.0 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 1/3] perf vendor-events: Fix all remaining invalid JSON files 2021-10-07 11:05 ` James Clark @ 2021-10-07 11:05 ` James Clark -1 siblings, 0 replies; 36+ messages in thread From: James Clark @ 2021-10-07 11:05 UTC (permalink / raw) To: acme, john.garry, ak, linux-perf-users Cc: Nick.Forrington, Andrew.Kilroy, James Clark, Will Deacon, Mathieu Poirier, Leo Yan, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim, linux-arm-kernel, linux-kernel Remove trailing commas. A later commit will make the parser more strict and these will not be valid anymore. Signed-off-by: James Clark <james.clark@arm.com> --- .../arch/arm64/ampere/emag/bus.json | 2 +- .../arch/arm64/ampere/emag/cache.json | 20 +++++++++---------- .../arch/arm64/ampere/emag/clock.json | 2 +- .../arch/arm64/ampere/emag/exception.json | 4 ++-- .../arch/arm64/ampere/emag/instruction.json | 10 +++++----- .../arch/arm64/ampere/emag/memory.json | 4 ++-- .../arch/arm64/hisilicon/hip08/metrics.json | 2 +- .../pmu-events/arch/nds32/n13/atcpmu.json | 2 +- .../pmu-events/arch/s390/cf_z10/basic.json | 2 +- .../pmu-events/arch/s390/cf_z10/crypto.json | 2 +- .../pmu-events/arch/s390/cf_z10/extended.json | 2 +- .../pmu-events/arch/s390/cf_z13/basic.json | 2 +- .../pmu-events/arch/s390/cf_z13/crypto.json | 2 +- .../pmu-events/arch/s390/cf_z13/extended.json | 2 +- .../pmu-events/arch/s390/cf_z14/basic.json | 2 +- .../pmu-events/arch/s390/cf_z14/crypto.json | 2 +- .../pmu-events/arch/s390/cf_z14/extended.json | 2 +- .../pmu-events/arch/s390/cf_z15/basic.json | 2 +- .../pmu-events/arch/s390/cf_z15/crypto.json | 2 +- .../pmu-events/arch/s390/cf_z15/crypto6.json | 2 +- .../pmu-events/arch/s390/cf_z15/extended.json | 2 +- .../pmu-events/arch/s390/cf_z196/basic.json | 2 +- .../pmu-events/arch/s390/cf_z196/crypto.json | 2 +- .../arch/s390/cf_z196/extended.json | 2 +- .../pmu-events/arch/s390/cf_zec12/basic.json | 2 +- .../pmu-events/arch/s390/cf_zec12/crypto.json | 2 +- .../arch/s390/cf_zec12/extended.json | 2 +- .../arch/test/test_soc/cpu/uncore.json | 2 +- .../arch/x86/icelakex/icx-metrics.json | 2 +- 29 files changed, 44 insertions(+), 44 deletions(-) diff --git a/tools/perf/pmu-events/arch/arm64/ampere/emag/bus.json b/tools/perf/pmu-events/arch/arm64/ampere/emag/bus.json index 9bea1ba1c4d2..cf48d0dfc759 100644 --- a/tools/perf/pmu-events/arch/arm64/ampere/emag/bus.json +++ b/tools/perf/pmu-events/arch/arm64/ampere/emag/bus.json @@ -18,6 +18,6 @@ "ArchStdEvent": "BUS_ACCESS_PERIPH" }, { - "ArchStdEvent": "BUS_ACCESS", + "ArchStdEvent": "BUS_ACCESS" } ] diff --git a/tools/perf/pmu-events/arch/arm64/ampere/emag/cache.json b/tools/perf/pmu-events/arch/arm64/ampere/emag/cache.json index 1e25f2ae4ae0..4cc50b7da526 100644 --- a/tools/perf/pmu-events/arch/arm64/ampere/emag/cache.json +++ b/tools/perf/pmu-events/arch/arm64/ampere/emag/cache.json @@ -39,31 +39,31 @@ "ArchStdEvent": "L2D_CACHE_INVAL" }, { - "ArchStdEvent": "L1I_CACHE_REFILL", + "ArchStdEvent": "L1I_CACHE_REFILL" }, { - "ArchStdEvent": "L1I_TLB_REFILL", + "ArchStdEvent": "L1I_TLB_REFILL" }, { - "ArchStdEvent": "L1D_CACHE_REFILL", + "ArchStdEvent": "L1D_CACHE_REFILL" }, { - "ArchStdEvent": "L1D_CACHE", + "ArchStdEvent": "L1D_CACHE" }, { - "ArchStdEvent": "L1D_TLB_REFILL", + "ArchStdEvent": "L1D_TLB_REFILL" }, { - "ArchStdEvent": "L1I_CACHE", + "ArchStdEvent": "L1I_CACHE" }, { - "ArchStdEvent": "L2D_CACHE", + "ArchStdEvent": "L2D_CACHE" }, { - "ArchStdEvent": "L2D_CACHE_REFILL", + "ArchStdEvent": "L2D_CACHE_REFILL" }, { - "ArchStdEvent": "L2D_CACHE_WB", + "ArchStdEvent": "L2D_CACHE_WB" }, { "PublicDescription": "This event counts any load or store operation which accesses the data L1 TLB", @@ -72,7 +72,7 @@ }, { "PublicDescription": "This event counts any instruction fetch which accesses the instruction L1 TLB", - "ArchStdEvent": "L1I_TLB", + "ArchStdEvent": "L1I_TLB" }, { "PublicDescription": "Level 2 access to data TLB that caused a page table walk. This event counts on any data access which causes L2D_TLB_REFILL to count", diff --git a/tools/perf/pmu-events/arch/arm64/ampere/emag/clock.json b/tools/perf/pmu-events/arch/arm64/ampere/emag/clock.json index 9076ca2daf9e..927a6f629a03 100644 --- a/tools/perf/pmu-events/arch/arm64/ampere/emag/clock.json +++ b/tools/perf/pmu-events/arch/arm64/ampere/emag/clock.json @@ -1,7 +1,7 @@ [ { "PublicDescription": "The number of core clock cycles", - "ArchStdEvent": "CPU_CYCLES", + "ArchStdEvent": "CPU_CYCLES" }, { "PublicDescription": "FSU clocking gated off cycle", diff --git a/tools/perf/pmu-events/arch/arm64/ampere/emag/exception.json b/tools/perf/pmu-events/arch/arm64/ampere/emag/exception.json index 9761433ad329..ada052e19632 100644 --- a/tools/perf/pmu-events/arch/arm64/ampere/emag/exception.json +++ b/tools/perf/pmu-events/arch/arm64/ampere/emag/exception.json @@ -36,9 +36,9 @@ "ArchStdEvent": "EXC_TRAP_FIQ" }, { - "ArchStdEvent": "EXC_TAKEN", + "ArchStdEvent": "EXC_TAKEN" }, { - "ArchStdEvent": "EXC_RETURN", + "ArchStdEvent": "EXC_RETURN" } ] diff --git a/tools/perf/pmu-events/arch/arm64/ampere/emag/instruction.json b/tools/perf/pmu-events/arch/arm64/ampere/emag/instruction.json index 482aa3f19e58..62f6276e3016 100644 --- a/tools/perf/pmu-events/arch/arm64/ampere/emag/instruction.json +++ b/tools/perf/pmu-events/arch/arm64/ampere/emag/instruction.json @@ -44,25 +44,25 @@ "BriefDescription": "Software increment" }, { - "ArchStdEvent": "INST_RETIRED", + "ArchStdEvent": "INST_RETIRED" }, { "ArchStdEvent": "CID_WRITE_RETIRED", "BriefDescription": "Write to CONTEXTIDR" }, { - "ArchStdEvent": "INST_SPEC", + "ArchStdEvent": "INST_SPEC" }, { - "ArchStdEvent": "TTBR_WRITE_RETIRED", + "ArchStdEvent": "TTBR_WRITE_RETIRED" }, { "PublicDescription": "This event counts all branches, taken or not. This excludes exception entries, debug entries and CCFAIL branches", - "ArchStdEvent": "BR_RETIRED", + "ArchStdEvent": "BR_RETIRED" }, { "PublicDescription": "This event counts any branch counted by BR_RETIRED which is not correctly predicted and causes a pipeline flush", - "ArchStdEvent": "BR_MIS_PRED_RETIRED", + "ArchStdEvent": "BR_MIS_PRED_RETIRED" }, { "PublicDescription": "Operation speculatively executed, NOP", diff --git a/tools/perf/pmu-events/arch/arm64/ampere/emag/memory.json b/tools/perf/pmu-events/arch/arm64/ampere/emag/memory.json index 2e7555696caf..50157e8c2005 100644 --- a/tools/perf/pmu-events/arch/arm64/ampere/emag/memory.json +++ b/tools/perf/pmu-events/arch/arm64/ampere/emag/memory.json @@ -15,10 +15,10 @@ "ArchStdEvent": "UNALIGNED_LDST_SPEC" }, { - "ArchStdEvent": "MEM_ACCESS", + "ArchStdEvent": "MEM_ACCESS" }, { "PublicDescription": "This event counts any correctable or uncorrectable memory error (ECC or parity) in the protected core RAMs", - "ArchStdEvent": "MEMORY_ERROR", + "ArchStdEvent": "MEMORY_ERROR" } ] diff --git a/tools/perf/pmu-events/arch/arm64/hisilicon/hip08/metrics.json b/tools/perf/pmu-events/arch/arm64/hisilicon/hip08/metrics.json index dda8e59149d2..6970203cb247 100644 --- a/tools/perf/pmu-events/arch/arm64/hisilicon/hip08/metrics.json +++ b/tools/perf/pmu-events/arch/arm64/hisilicon/hip08/metrics.json @@ -229,5 +229,5 @@ "BriefDescription": "Store bound L3 topdown metric", "MetricGroup": "TopDownL3", "MetricName": "store_bound" - }, + } ] diff --git a/tools/perf/pmu-events/arch/nds32/n13/atcpmu.json b/tools/perf/pmu-events/arch/nds32/n13/atcpmu.json index 5347350c360c..3e7ac409d894 100644 --- a/tools/perf/pmu-events/arch/nds32/n13/atcpmu.json +++ b/tools/perf/pmu-events/arch/nds32/n13/atcpmu.json @@ -286,5 +286,5 @@ "EventCode": "0x21e", "EventName": "pop25_inst", "BriefDescription": "V3 POP25 instructions" - }, + } ] diff --git a/tools/perf/pmu-events/arch/s390/cf_z10/basic.json b/tools/perf/pmu-events/arch/s390/cf_z10/basic.json index 2dd8dafff2ef..783de7f1aeaa 100644 --- a/tools/perf/pmu-events/arch/s390/cf_z10/basic.json +++ b/tools/perf/pmu-events/arch/s390/cf_z10/basic.json @@ -82,5 +82,5 @@ "EventName": "PROBLEM_STATE_L1D_PENALTY_CYCLES", "BriefDescription": "Problem-State L1D Penalty Cycles", "PublicDescription": "Problem-State Level-1 D-Cache Penalty Cycle Count" - }, + } ] diff --git a/tools/perf/pmu-events/arch/s390/cf_z10/crypto.json b/tools/perf/pmu-events/arch/s390/cf_z10/crypto.json index db286f19e7b6..3f28007d3892 100644 --- a/tools/perf/pmu-events/arch/s390/cf_z10/crypto.json +++ b/tools/perf/pmu-events/arch/s390/cf_z10/crypto.json @@ -110,5 +110,5 @@ "EventName": "AES_BLOCKED_CYCLES", "BriefDescription": "AES Blocked Cycles", "PublicDescription": "Total number of CPU cycles blocked for the AES functions issued by the CPU because the DEA/AES coprocessor is busy performing a function issued by another CPU" - }, + } ] diff --git a/tools/perf/pmu-events/arch/s390/cf_z10/extended.json b/tools/perf/pmu-events/arch/s390/cf_z10/extended.json index b6b7f29ca831..86bd8ba9391d 100644 --- a/tools/perf/pmu-events/arch/s390/cf_z10/extended.json +++ b/tools/perf/pmu-events/arch/s390/cf_z10/extended.json @@ -124,5 +124,5 @@ "EventName": "L2C_STORES_SENT", "BriefDescription": "L2C Stores Sent", "PublicDescription": "Incremented by one for every store sent to Level-2 (L1.5) cache" - }, + } ] diff --git a/tools/perf/pmu-events/arch/s390/cf_z13/basic.json b/tools/perf/pmu-events/arch/s390/cf_z13/basic.json index 2dd8dafff2ef..783de7f1aeaa 100644 --- a/tools/perf/pmu-events/arch/s390/cf_z13/basic.json +++ b/tools/perf/pmu-events/arch/s390/cf_z13/basic.json @@ -82,5 +82,5 @@ "EventName": "PROBLEM_STATE_L1D_PENALTY_CYCLES", "BriefDescription": "Problem-State L1D Penalty Cycles", "PublicDescription": "Problem-State Level-1 D-Cache Penalty Cycle Count" - }, + } ] diff --git a/tools/perf/pmu-events/arch/s390/cf_z13/crypto.json b/tools/perf/pmu-events/arch/s390/cf_z13/crypto.json index db286f19e7b6..3f28007d3892 100644 --- a/tools/perf/pmu-events/arch/s390/cf_z13/crypto.json +++ b/tools/perf/pmu-events/arch/s390/cf_z13/crypto.json @@ -110,5 +110,5 @@ "EventName": "AES_BLOCKED_CYCLES", "BriefDescription": "AES Blocked Cycles", "PublicDescription": "Total number of CPU cycles blocked for the AES functions issued by the CPU because the DEA/AES coprocessor is busy performing a function issued by another CPU" - }, + } ] diff --git a/tools/perf/pmu-events/arch/s390/cf_z13/extended.json b/tools/perf/pmu-events/arch/s390/cf_z13/extended.json index 5da8296b667e..1a5e4f89c57e 100644 --- a/tools/perf/pmu-events/arch/s390/cf_z13/extended.json +++ b/tools/perf/pmu-events/arch/s390/cf_z13/extended.json @@ -390,5 +390,5 @@ "EventName": "MT_DIAG_CYCLES_TWO_THR_ACTIVE", "BriefDescription": "Cycle count with two threads active", "PublicDescription": "Cycle count with two threads active" - }, + } ] diff --git a/tools/perf/pmu-events/arch/s390/cf_z14/basic.json b/tools/perf/pmu-events/arch/s390/cf_z14/basic.json index 17fb5241928b..fc762e9f1d6e 100644 --- a/tools/perf/pmu-events/arch/s390/cf_z14/basic.json +++ b/tools/perf/pmu-events/arch/s390/cf_z14/basic.json @@ -54,5 +54,5 @@ "EventName": "PROBLEM_STATE_INSTRUCTIONS", "BriefDescription": "Problem-State Instructions", "PublicDescription": "Problem-State Instruction Count" - }, + } ] diff --git a/tools/perf/pmu-events/arch/s390/cf_z14/crypto.json b/tools/perf/pmu-events/arch/s390/cf_z14/crypto.json index db286f19e7b6..3f28007d3892 100644 --- a/tools/perf/pmu-events/arch/s390/cf_z14/crypto.json +++ b/tools/perf/pmu-events/arch/s390/cf_z14/crypto.json @@ -110,5 +110,5 @@ "EventName": "AES_BLOCKED_CYCLES", "BriefDescription": "AES Blocked Cycles", "PublicDescription": "Total number of CPU cycles blocked for the AES functions issued by the CPU because the DEA/AES coprocessor is busy performing a function issued by another CPU" - }, + } ] diff --git a/tools/perf/pmu-events/arch/s390/cf_z14/extended.json b/tools/perf/pmu-events/arch/s390/cf_z14/extended.json index 89e070727e1b..4942b20a1ea1 100644 --- a/tools/perf/pmu-events/arch/s390/cf_z14/extended.json +++ b/tools/perf/pmu-events/arch/s390/cf_z14/extended.json @@ -369,5 +369,5 @@ "EventName": "MT_DIAG_CYCLES_TWO_THR_ACTIVE", "BriefDescription": "Cycle count with two threads active", "PublicDescription": "Cycle count with two threads active" - }, + } ] diff --git a/tools/perf/pmu-events/arch/s390/cf_z15/basic.json b/tools/perf/pmu-events/arch/s390/cf_z15/basic.json index 17fb5241928b..fc762e9f1d6e 100644 --- a/tools/perf/pmu-events/arch/s390/cf_z15/basic.json +++ b/tools/perf/pmu-events/arch/s390/cf_z15/basic.json @@ -54,5 +54,5 @@ "EventName": "PROBLEM_STATE_INSTRUCTIONS", "BriefDescription": "Problem-State Instructions", "PublicDescription": "Problem-State Instruction Count" - }, + } ] diff --git a/tools/perf/pmu-events/arch/s390/cf_z15/crypto.json b/tools/perf/pmu-events/arch/s390/cf_z15/crypto.json index db286f19e7b6..3f28007d3892 100644 --- a/tools/perf/pmu-events/arch/s390/cf_z15/crypto.json +++ b/tools/perf/pmu-events/arch/s390/cf_z15/crypto.json @@ -110,5 +110,5 @@ "EventName": "AES_BLOCKED_CYCLES", "BriefDescription": "AES Blocked Cycles", "PublicDescription": "Total number of CPU cycles blocked for the AES functions issued by the CPU because the DEA/AES coprocessor is busy performing a function issued by another CPU" - }, + } ] diff --git a/tools/perf/pmu-events/arch/s390/cf_z15/crypto6.json b/tools/perf/pmu-events/arch/s390/cf_z15/crypto6.json index c998e4f1d1d2..ad79189050a0 100644 --- a/tools/perf/pmu-events/arch/s390/cf_z15/crypto6.json +++ b/tools/perf/pmu-events/arch/s390/cf_z15/crypto6.json @@ -26,5 +26,5 @@ "EventName": "ECC_BLOCKED_CYCLES_COUNT", "BriefDescription": "ECC Blocked Cycles Count", "PublicDescription": "This counter counts the total number of CPU cycles blocked for the elliptic-curve cryptography (ECC) functions issued by the CPU because the ECC coprocessor is busy performing a function issued by another CPU." - }, + } ] diff --git a/tools/perf/pmu-events/arch/s390/cf_z15/extended.json b/tools/perf/pmu-events/arch/s390/cf_z15/extended.json index 24c4ba2a9ae5..8ac61f8f286b 100644 --- a/tools/perf/pmu-events/arch/s390/cf_z15/extended.json +++ b/tools/perf/pmu-events/arch/s390/cf_z15/extended.json @@ -397,5 +397,5 @@ "EventName": "MT_DIAG_CYCLES_TWO_THR_ACTIVE", "BriefDescription": "Cycle count with two threads active", "PublicDescription": "Cycle count with two threads active" - }, + } ] diff --git a/tools/perf/pmu-events/arch/s390/cf_z196/basic.json b/tools/perf/pmu-events/arch/s390/cf_z196/basic.json index 2dd8dafff2ef..783de7f1aeaa 100644 --- a/tools/perf/pmu-events/arch/s390/cf_z196/basic.json +++ b/tools/perf/pmu-events/arch/s390/cf_z196/basic.json @@ -82,5 +82,5 @@ "EventName": "PROBLEM_STATE_L1D_PENALTY_CYCLES", "BriefDescription": "Problem-State L1D Penalty Cycles", "PublicDescription": "Problem-State Level-1 D-Cache Penalty Cycle Count" - }, + } ] diff --git a/tools/perf/pmu-events/arch/s390/cf_z196/crypto.json b/tools/perf/pmu-events/arch/s390/cf_z196/crypto.json index db286f19e7b6..3f28007d3892 100644 --- a/tools/perf/pmu-events/arch/s390/cf_z196/crypto.json +++ b/tools/perf/pmu-events/arch/s390/cf_z196/crypto.json @@ -110,5 +110,5 @@ "EventName": "AES_BLOCKED_CYCLES", "BriefDescription": "AES Blocked Cycles", "PublicDescription": "Total number of CPU cycles blocked for the AES functions issued by the CPU because the DEA/AES coprocessor is busy performing a function issued by another CPU" - }, + } ] diff --git a/tools/perf/pmu-events/arch/s390/cf_z196/extended.json b/tools/perf/pmu-events/arch/s390/cf_z196/extended.json index b7b42a870bb0..86b29fd181cf 100644 --- a/tools/perf/pmu-events/arch/s390/cf_z196/extended.json +++ b/tools/perf/pmu-events/arch/s390/cf_z196/extended.json @@ -166,5 +166,5 @@ "EventName": "L1I_OFFCHIP_L3_SOURCED_WRITES", "BriefDescription": "L1I Off-Chip L3 Sourced Writes", "PublicDescription": "A directory write to the Level-1 I-Cache directory where the returned cache line was sourced from an Off Chip/On Book Level-3 cache" - }, + } ] diff --git a/tools/perf/pmu-events/arch/s390/cf_zec12/basic.json b/tools/perf/pmu-events/arch/s390/cf_zec12/basic.json index 2dd8dafff2ef..783de7f1aeaa 100644 --- a/tools/perf/pmu-events/arch/s390/cf_zec12/basic.json +++ b/tools/perf/pmu-events/arch/s390/cf_zec12/basic.json @@ -82,5 +82,5 @@ "EventName": "PROBLEM_STATE_L1D_PENALTY_CYCLES", "BriefDescription": "Problem-State L1D Penalty Cycles", "PublicDescription": "Problem-State Level-1 D-Cache Penalty Cycle Count" - }, + } ] diff --git a/tools/perf/pmu-events/arch/s390/cf_zec12/crypto.json b/tools/perf/pmu-events/arch/s390/cf_zec12/crypto.json index db286f19e7b6..3f28007d3892 100644 --- a/tools/perf/pmu-events/arch/s390/cf_zec12/crypto.json +++ b/tools/perf/pmu-events/arch/s390/cf_zec12/crypto.json @@ -110,5 +110,5 @@ "EventName": "AES_BLOCKED_CYCLES", "BriefDescription": "AES Blocked Cycles", "PublicDescription": "Total number of CPU cycles blocked for the AES functions issued by the CPU because the DEA/AES coprocessor is busy performing a function issued by another CPU" - }, + } ] diff --git a/tools/perf/pmu-events/arch/s390/cf_zec12/extended.json b/tools/perf/pmu-events/arch/s390/cf_zec12/extended.json index 162251037219..f40cbed89418 100644 --- a/tools/perf/pmu-events/arch/s390/cf_zec12/extended.json +++ b/tools/perf/pmu-events/arch/s390/cf_zec12/extended.json @@ -243,5 +243,5 @@ "EventName": "TX_C_TABORT_SPECIAL", "BriefDescription": "Aborted transactions in constrained TX mode using special completion logic", "PublicDescription": "A transaction abort has occurred in a constrained transactional-execution mode and the CPU is using special logic to allow the transaction to complete" - }, + } ] diff --git a/tools/perf/pmu-events/arch/test/test_soc/cpu/uncore.json b/tools/perf/pmu-events/arch/test/test_soc/cpu/uncore.json index 788766f45dbc..73089c682f80 100644 --- a/tools/perf/pmu-events/arch/test/test_soc/cpu/uncore.json +++ b/tools/perf/pmu-events/arch/test/test_soc/cpu/uncore.json @@ -38,5 +38,5 @@ "BriefDescription": "Total cache hits", "PublicDescription": "Total cache hits", "Unit": "imc" - }, + } ] diff --git a/tools/perf/pmu-events/arch/x86/icelakex/icx-metrics.json b/tools/perf/pmu-events/arch/x86/icelakex/icx-metrics.json index 57ddbb9f9b31..14b9a8ab15b9 100644 --- a/tools/perf/pmu-events/arch/x86/icelakex/icx-metrics.json +++ b/tools/perf/pmu-events/arch/x86/icelakex/icx-metrics.json @@ -311,5 +311,5 @@ "MetricExpr": "(cstate_pkg@c6\\-residency@ / msr@tsc@) * 100", "MetricGroup": "Power", "MetricName": "C6_Pkg_Residency" - }, + } ] -- 2.28.0 ^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH 1/3] perf vendor-events: Fix all remaining invalid JSON files @ 2021-10-07 11:05 ` James Clark 0 siblings, 0 replies; 36+ messages in thread From: James Clark @ 2021-10-07 11:05 UTC (permalink / raw) To: acme, john.garry, ak, linux-perf-users Cc: Nick.Forrington, Andrew.Kilroy, James Clark, Will Deacon, Mathieu Poirier, Leo Yan, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim, linux-arm-kernel, linux-kernel Remove trailing commas. A later commit will make the parser more strict and these will not be valid anymore. Signed-off-by: James Clark <james.clark@arm.com> --- .../arch/arm64/ampere/emag/bus.json | 2 +- .../arch/arm64/ampere/emag/cache.json | 20 +++++++++---------- .../arch/arm64/ampere/emag/clock.json | 2 +- .../arch/arm64/ampere/emag/exception.json | 4 ++-- .../arch/arm64/ampere/emag/instruction.json | 10 +++++----- .../arch/arm64/ampere/emag/memory.json | 4 ++-- .../arch/arm64/hisilicon/hip08/metrics.json | 2 +- .../pmu-events/arch/nds32/n13/atcpmu.json | 2 +- .../pmu-events/arch/s390/cf_z10/basic.json | 2 +- .../pmu-events/arch/s390/cf_z10/crypto.json | 2 +- .../pmu-events/arch/s390/cf_z10/extended.json | 2 +- .../pmu-events/arch/s390/cf_z13/basic.json | 2 +- .../pmu-events/arch/s390/cf_z13/crypto.json | 2 +- .../pmu-events/arch/s390/cf_z13/extended.json | 2 +- .../pmu-events/arch/s390/cf_z14/basic.json | 2 +- .../pmu-events/arch/s390/cf_z14/crypto.json | 2 +- .../pmu-events/arch/s390/cf_z14/extended.json | 2 +- .../pmu-events/arch/s390/cf_z15/basic.json | 2 +- .../pmu-events/arch/s390/cf_z15/crypto.json | 2 +- .../pmu-events/arch/s390/cf_z15/crypto6.json | 2 +- .../pmu-events/arch/s390/cf_z15/extended.json | 2 +- .../pmu-events/arch/s390/cf_z196/basic.json | 2 +- .../pmu-events/arch/s390/cf_z196/crypto.json | 2 +- .../arch/s390/cf_z196/extended.json | 2 +- .../pmu-events/arch/s390/cf_zec12/basic.json | 2 +- .../pmu-events/arch/s390/cf_zec12/crypto.json | 2 +- .../arch/s390/cf_zec12/extended.json | 2 +- .../arch/test/test_soc/cpu/uncore.json | 2 +- .../arch/x86/icelakex/icx-metrics.json | 2 +- 29 files changed, 44 insertions(+), 44 deletions(-) diff --git a/tools/perf/pmu-events/arch/arm64/ampere/emag/bus.json b/tools/perf/pmu-events/arch/arm64/ampere/emag/bus.json index 9bea1ba1c4d2..cf48d0dfc759 100644 --- a/tools/perf/pmu-events/arch/arm64/ampere/emag/bus.json +++ b/tools/perf/pmu-events/arch/arm64/ampere/emag/bus.json @@ -18,6 +18,6 @@ "ArchStdEvent": "BUS_ACCESS_PERIPH" }, { - "ArchStdEvent": "BUS_ACCESS", + "ArchStdEvent": "BUS_ACCESS" } ] diff --git a/tools/perf/pmu-events/arch/arm64/ampere/emag/cache.json b/tools/perf/pmu-events/arch/arm64/ampere/emag/cache.json index 1e25f2ae4ae0..4cc50b7da526 100644 --- a/tools/perf/pmu-events/arch/arm64/ampere/emag/cache.json +++ b/tools/perf/pmu-events/arch/arm64/ampere/emag/cache.json @@ -39,31 +39,31 @@ "ArchStdEvent": "L2D_CACHE_INVAL" }, { - "ArchStdEvent": "L1I_CACHE_REFILL", + "ArchStdEvent": "L1I_CACHE_REFILL" }, { - "ArchStdEvent": "L1I_TLB_REFILL", + "ArchStdEvent": "L1I_TLB_REFILL" }, { - "ArchStdEvent": "L1D_CACHE_REFILL", + "ArchStdEvent": "L1D_CACHE_REFILL" }, { - "ArchStdEvent": "L1D_CACHE", + "ArchStdEvent": "L1D_CACHE" }, { - "ArchStdEvent": "L1D_TLB_REFILL", + "ArchStdEvent": "L1D_TLB_REFILL" }, { - "ArchStdEvent": "L1I_CACHE", + "ArchStdEvent": "L1I_CACHE" }, { - "ArchStdEvent": "L2D_CACHE", + "ArchStdEvent": "L2D_CACHE" }, { - "ArchStdEvent": "L2D_CACHE_REFILL", + "ArchStdEvent": "L2D_CACHE_REFILL" }, { - "ArchStdEvent": "L2D_CACHE_WB", + "ArchStdEvent": "L2D_CACHE_WB" }, { "PublicDescription": "This event counts any load or store operation which accesses the data L1 TLB", @@ -72,7 +72,7 @@ }, { "PublicDescription": "This event counts any instruction fetch which accesses the instruction L1 TLB", - "ArchStdEvent": "L1I_TLB", + "ArchStdEvent": "L1I_TLB" }, { "PublicDescription": "Level 2 access to data TLB that caused a page table walk. This event counts on any data access which causes L2D_TLB_REFILL to count", diff --git a/tools/perf/pmu-events/arch/arm64/ampere/emag/clock.json b/tools/perf/pmu-events/arch/arm64/ampere/emag/clock.json index 9076ca2daf9e..927a6f629a03 100644 --- a/tools/perf/pmu-events/arch/arm64/ampere/emag/clock.json +++ b/tools/perf/pmu-events/arch/arm64/ampere/emag/clock.json @@ -1,7 +1,7 @@ [ { "PublicDescription": "The number of core clock cycles", - "ArchStdEvent": "CPU_CYCLES", + "ArchStdEvent": "CPU_CYCLES" }, { "PublicDescription": "FSU clocking gated off cycle", diff --git a/tools/perf/pmu-events/arch/arm64/ampere/emag/exception.json b/tools/perf/pmu-events/arch/arm64/ampere/emag/exception.json index 9761433ad329..ada052e19632 100644 --- a/tools/perf/pmu-events/arch/arm64/ampere/emag/exception.json +++ b/tools/perf/pmu-events/arch/arm64/ampere/emag/exception.json @@ -36,9 +36,9 @@ "ArchStdEvent": "EXC_TRAP_FIQ" }, { - "ArchStdEvent": "EXC_TAKEN", + "ArchStdEvent": "EXC_TAKEN" }, { - "ArchStdEvent": "EXC_RETURN", + "ArchStdEvent": "EXC_RETURN" } ] diff --git a/tools/perf/pmu-events/arch/arm64/ampere/emag/instruction.json b/tools/perf/pmu-events/arch/arm64/ampere/emag/instruction.json index 482aa3f19e58..62f6276e3016 100644 --- a/tools/perf/pmu-events/arch/arm64/ampere/emag/instruction.json +++ b/tools/perf/pmu-events/arch/arm64/ampere/emag/instruction.json @@ -44,25 +44,25 @@ "BriefDescription": "Software increment" }, { - "ArchStdEvent": "INST_RETIRED", + "ArchStdEvent": "INST_RETIRED" }, { "ArchStdEvent": "CID_WRITE_RETIRED", "BriefDescription": "Write to CONTEXTIDR" }, { - "ArchStdEvent": "INST_SPEC", + "ArchStdEvent": "INST_SPEC" }, { - "ArchStdEvent": "TTBR_WRITE_RETIRED", + "ArchStdEvent": "TTBR_WRITE_RETIRED" }, { "PublicDescription": "This event counts all branches, taken or not. This excludes exception entries, debug entries and CCFAIL branches", - "ArchStdEvent": "BR_RETIRED", + "ArchStdEvent": "BR_RETIRED" }, { "PublicDescription": "This event counts any branch counted by BR_RETIRED which is not correctly predicted and causes a pipeline flush", - "ArchStdEvent": "BR_MIS_PRED_RETIRED", + "ArchStdEvent": "BR_MIS_PRED_RETIRED" }, { "PublicDescription": "Operation speculatively executed, NOP", diff --git a/tools/perf/pmu-events/arch/arm64/ampere/emag/memory.json b/tools/perf/pmu-events/arch/arm64/ampere/emag/memory.json index 2e7555696caf..50157e8c2005 100644 --- a/tools/perf/pmu-events/arch/arm64/ampere/emag/memory.json +++ b/tools/perf/pmu-events/arch/arm64/ampere/emag/memory.json @@ -15,10 +15,10 @@ "ArchStdEvent": "UNALIGNED_LDST_SPEC" }, { - "ArchStdEvent": "MEM_ACCESS", + "ArchStdEvent": "MEM_ACCESS" }, { "PublicDescription": "This event counts any correctable or uncorrectable memory error (ECC or parity) in the protected core RAMs", - "ArchStdEvent": "MEMORY_ERROR", + "ArchStdEvent": "MEMORY_ERROR" } ] diff --git a/tools/perf/pmu-events/arch/arm64/hisilicon/hip08/metrics.json b/tools/perf/pmu-events/arch/arm64/hisilicon/hip08/metrics.json index dda8e59149d2..6970203cb247 100644 --- a/tools/perf/pmu-events/arch/arm64/hisilicon/hip08/metrics.json +++ b/tools/perf/pmu-events/arch/arm64/hisilicon/hip08/metrics.json @@ -229,5 +229,5 @@ "BriefDescription": "Store bound L3 topdown metric", "MetricGroup": "TopDownL3", "MetricName": "store_bound" - }, + } ] diff --git a/tools/perf/pmu-events/arch/nds32/n13/atcpmu.json b/tools/perf/pmu-events/arch/nds32/n13/atcpmu.json index 5347350c360c..3e7ac409d894 100644 --- a/tools/perf/pmu-events/arch/nds32/n13/atcpmu.json +++ b/tools/perf/pmu-events/arch/nds32/n13/atcpmu.json @@ -286,5 +286,5 @@ "EventCode": "0x21e", "EventName": "pop25_inst", "BriefDescription": "V3 POP25 instructions" - }, + } ] diff --git a/tools/perf/pmu-events/arch/s390/cf_z10/basic.json b/tools/perf/pmu-events/arch/s390/cf_z10/basic.json index 2dd8dafff2ef..783de7f1aeaa 100644 --- a/tools/perf/pmu-events/arch/s390/cf_z10/basic.json +++ b/tools/perf/pmu-events/arch/s390/cf_z10/basic.json @@ -82,5 +82,5 @@ "EventName": "PROBLEM_STATE_L1D_PENALTY_CYCLES", "BriefDescription": "Problem-State L1D Penalty Cycles", "PublicDescription": "Problem-State Level-1 D-Cache Penalty Cycle Count" - }, + } ] diff --git a/tools/perf/pmu-events/arch/s390/cf_z10/crypto.json b/tools/perf/pmu-events/arch/s390/cf_z10/crypto.json index db286f19e7b6..3f28007d3892 100644 --- a/tools/perf/pmu-events/arch/s390/cf_z10/crypto.json +++ b/tools/perf/pmu-events/arch/s390/cf_z10/crypto.json @@ -110,5 +110,5 @@ "EventName": "AES_BLOCKED_CYCLES", "BriefDescription": "AES Blocked Cycles", "PublicDescription": "Total number of CPU cycles blocked for the AES functions issued by the CPU because the DEA/AES coprocessor is busy performing a function issued by another CPU" - }, + } ] diff --git a/tools/perf/pmu-events/arch/s390/cf_z10/extended.json b/tools/perf/pmu-events/arch/s390/cf_z10/extended.json index b6b7f29ca831..86bd8ba9391d 100644 --- a/tools/perf/pmu-events/arch/s390/cf_z10/extended.json +++ b/tools/perf/pmu-events/arch/s390/cf_z10/extended.json @@ -124,5 +124,5 @@ "EventName": "L2C_STORES_SENT", "BriefDescription": "L2C Stores Sent", "PublicDescription": "Incremented by one for every store sent to Level-2 (L1.5) cache" - }, + } ] diff --git a/tools/perf/pmu-events/arch/s390/cf_z13/basic.json b/tools/perf/pmu-events/arch/s390/cf_z13/basic.json index 2dd8dafff2ef..783de7f1aeaa 100644 --- a/tools/perf/pmu-events/arch/s390/cf_z13/basic.json +++ b/tools/perf/pmu-events/arch/s390/cf_z13/basic.json @@ -82,5 +82,5 @@ "EventName": "PROBLEM_STATE_L1D_PENALTY_CYCLES", "BriefDescription": "Problem-State L1D Penalty Cycles", "PublicDescription": "Problem-State Level-1 D-Cache Penalty Cycle Count" - }, + } ] diff --git a/tools/perf/pmu-events/arch/s390/cf_z13/crypto.json b/tools/perf/pmu-events/arch/s390/cf_z13/crypto.json index db286f19e7b6..3f28007d3892 100644 --- a/tools/perf/pmu-events/arch/s390/cf_z13/crypto.json +++ b/tools/perf/pmu-events/arch/s390/cf_z13/crypto.json @@ -110,5 +110,5 @@ "EventName": "AES_BLOCKED_CYCLES", "BriefDescription": "AES Blocked Cycles", "PublicDescription": "Total number of CPU cycles blocked for the AES functions issued by the CPU because the DEA/AES coprocessor is busy performing a function issued by another CPU" - }, + } ] diff --git a/tools/perf/pmu-events/arch/s390/cf_z13/extended.json b/tools/perf/pmu-events/arch/s390/cf_z13/extended.json index 5da8296b667e..1a5e4f89c57e 100644 --- a/tools/perf/pmu-events/arch/s390/cf_z13/extended.json +++ b/tools/perf/pmu-events/arch/s390/cf_z13/extended.json @@ -390,5 +390,5 @@ "EventName": "MT_DIAG_CYCLES_TWO_THR_ACTIVE", "BriefDescription": "Cycle count with two threads active", "PublicDescription": "Cycle count with two threads active" - }, + } ] diff --git a/tools/perf/pmu-events/arch/s390/cf_z14/basic.json b/tools/perf/pmu-events/arch/s390/cf_z14/basic.json index 17fb5241928b..fc762e9f1d6e 100644 --- a/tools/perf/pmu-events/arch/s390/cf_z14/basic.json +++ b/tools/perf/pmu-events/arch/s390/cf_z14/basic.json @@ -54,5 +54,5 @@ "EventName": "PROBLEM_STATE_INSTRUCTIONS", "BriefDescription": "Problem-State Instructions", "PublicDescription": "Problem-State Instruction Count" - }, + } ] diff --git a/tools/perf/pmu-events/arch/s390/cf_z14/crypto.json b/tools/perf/pmu-events/arch/s390/cf_z14/crypto.json index db286f19e7b6..3f28007d3892 100644 --- a/tools/perf/pmu-events/arch/s390/cf_z14/crypto.json +++ b/tools/perf/pmu-events/arch/s390/cf_z14/crypto.json @@ -110,5 +110,5 @@ "EventName": "AES_BLOCKED_CYCLES", "BriefDescription": "AES Blocked Cycles", "PublicDescription": "Total number of CPU cycles blocked for the AES functions issued by the CPU because the DEA/AES coprocessor is busy performing a function issued by another CPU" - }, + } ] diff --git a/tools/perf/pmu-events/arch/s390/cf_z14/extended.json b/tools/perf/pmu-events/arch/s390/cf_z14/extended.json index 89e070727e1b..4942b20a1ea1 100644 --- a/tools/perf/pmu-events/arch/s390/cf_z14/extended.json +++ b/tools/perf/pmu-events/arch/s390/cf_z14/extended.json @@ -369,5 +369,5 @@ "EventName": "MT_DIAG_CYCLES_TWO_THR_ACTIVE", "BriefDescription": "Cycle count with two threads active", "PublicDescription": "Cycle count with two threads active" - }, + } ] diff --git a/tools/perf/pmu-events/arch/s390/cf_z15/basic.json b/tools/perf/pmu-events/arch/s390/cf_z15/basic.json index 17fb5241928b..fc762e9f1d6e 100644 --- a/tools/perf/pmu-events/arch/s390/cf_z15/basic.json +++ b/tools/perf/pmu-events/arch/s390/cf_z15/basic.json @@ -54,5 +54,5 @@ "EventName": "PROBLEM_STATE_INSTRUCTIONS", "BriefDescription": "Problem-State Instructions", "PublicDescription": "Problem-State Instruction Count" - }, + } ] diff --git a/tools/perf/pmu-events/arch/s390/cf_z15/crypto.json b/tools/perf/pmu-events/arch/s390/cf_z15/crypto.json index db286f19e7b6..3f28007d3892 100644 --- a/tools/perf/pmu-events/arch/s390/cf_z15/crypto.json +++ b/tools/perf/pmu-events/arch/s390/cf_z15/crypto.json @@ -110,5 +110,5 @@ "EventName": "AES_BLOCKED_CYCLES", "BriefDescription": "AES Blocked Cycles", "PublicDescription": "Total number of CPU cycles blocked for the AES functions issued by the CPU because the DEA/AES coprocessor is busy performing a function issued by another CPU" - }, + } ] diff --git a/tools/perf/pmu-events/arch/s390/cf_z15/crypto6.json b/tools/perf/pmu-events/arch/s390/cf_z15/crypto6.json index c998e4f1d1d2..ad79189050a0 100644 --- a/tools/perf/pmu-events/arch/s390/cf_z15/crypto6.json +++ b/tools/perf/pmu-events/arch/s390/cf_z15/crypto6.json @@ -26,5 +26,5 @@ "EventName": "ECC_BLOCKED_CYCLES_COUNT", "BriefDescription": "ECC Blocked Cycles Count", "PublicDescription": "This counter counts the total number of CPU cycles blocked for the elliptic-curve cryptography (ECC) functions issued by the CPU because the ECC coprocessor is busy performing a function issued by another CPU." - }, + } ] diff --git a/tools/perf/pmu-events/arch/s390/cf_z15/extended.json b/tools/perf/pmu-events/arch/s390/cf_z15/extended.json index 24c4ba2a9ae5..8ac61f8f286b 100644 --- a/tools/perf/pmu-events/arch/s390/cf_z15/extended.json +++ b/tools/perf/pmu-events/arch/s390/cf_z15/extended.json @@ -397,5 +397,5 @@ "EventName": "MT_DIAG_CYCLES_TWO_THR_ACTIVE", "BriefDescription": "Cycle count with two threads active", "PublicDescription": "Cycle count with two threads active" - }, + } ] diff --git a/tools/perf/pmu-events/arch/s390/cf_z196/basic.json b/tools/perf/pmu-events/arch/s390/cf_z196/basic.json index 2dd8dafff2ef..783de7f1aeaa 100644 --- a/tools/perf/pmu-events/arch/s390/cf_z196/basic.json +++ b/tools/perf/pmu-events/arch/s390/cf_z196/basic.json @@ -82,5 +82,5 @@ "EventName": "PROBLEM_STATE_L1D_PENALTY_CYCLES", "BriefDescription": "Problem-State L1D Penalty Cycles", "PublicDescription": "Problem-State Level-1 D-Cache Penalty Cycle Count" - }, + } ] diff --git a/tools/perf/pmu-events/arch/s390/cf_z196/crypto.json b/tools/perf/pmu-events/arch/s390/cf_z196/crypto.json index db286f19e7b6..3f28007d3892 100644 --- a/tools/perf/pmu-events/arch/s390/cf_z196/crypto.json +++ b/tools/perf/pmu-events/arch/s390/cf_z196/crypto.json @@ -110,5 +110,5 @@ "EventName": "AES_BLOCKED_CYCLES", "BriefDescription": "AES Blocked Cycles", "PublicDescription": "Total number of CPU cycles blocked for the AES functions issued by the CPU because the DEA/AES coprocessor is busy performing a function issued by another CPU" - }, + } ] diff --git a/tools/perf/pmu-events/arch/s390/cf_z196/extended.json b/tools/perf/pmu-events/arch/s390/cf_z196/extended.json index b7b42a870bb0..86b29fd181cf 100644 --- a/tools/perf/pmu-events/arch/s390/cf_z196/extended.json +++ b/tools/perf/pmu-events/arch/s390/cf_z196/extended.json @@ -166,5 +166,5 @@ "EventName": "L1I_OFFCHIP_L3_SOURCED_WRITES", "BriefDescription": "L1I Off-Chip L3 Sourced Writes", "PublicDescription": "A directory write to the Level-1 I-Cache directory where the returned cache line was sourced from an Off Chip/On Book Level-3 cache" - }, + } ] diff --git a/tools/perf/pmu-events/arch/s390/cf_zec12/basic.json b/tools/perf/pmu-events/arch/s390/cf_zec12/basic.json index 2dd8dafff2ef..783de7f1aeaa 100644 --- a/tools/perf/pmu-events/arch/s390/cf_zec12/basic.json +++ b/tools/perf/pmu-events/arch/s390/cf_zec12/basic.json @@ -82,5 +82,5 @@ "EventName": "PROBLEM_STATE_L1D_PENALTY_CYCLES", "BriefDescription": "Problem-State L1D Penalty Cycles", "PublicDescription": "Problem-State Level-1 D-Cache Penalty Cycle Count" - }, + } ] diff --git a/tools/perf/pmu-events/arch/s390/cf_zec12/crypto.json b/tools/perf/pmu-events/arch/s390/cf_zec12/crypto.json index db286f19e7b6..3f28007d3892 100644 --- a/tools/perf/pmu-events/arch/s390/cf_zec12/crypto.json +++ b/tools/perf/pmu-events/arch/s390/cf_zec12/crypto.json @@ -110,5 +110,5 @@ "EventName": "AES_BLOCKED_CYCLES", "BriefDescription": "AES Blocked Cycles", "PublicDescription": "Total number of CPU cycles blocked for the AES functions issued by the CPU because the DEA/AES coprocessor is busy performing a function issued by another CPU" - }, + } ] diff --git a/tools/perf/pmu-events/arch/s390/cf_zec12/extended.json b/tools/perf/pmu-events/arch/s390/cf_zec12/extended.json index 162251037219..f40cbed89418 100644 --- a/tools/perf/pmu-events/arch/s390/cf_zec12/extended.json +++ b/tools/perf/pmu-events/arch/s390/cf_zec12/extended.json @@ -243,5 +243,5 @@ "EventName": "TX_C_TABORT_SPECIAL", "BriefDescription": "Aborted transactions in constrained TX mode using special completion logic", "PublicDescription": "A transaction abort has occurred in a constrained transactional-execution mode and the CPU is using special logic to allow the transaction to complete" - }, + } ] diff --git a/tools/perf/pmu-events/arch/test/test_soc/cpu/uncore.json b/tools/perf/pmu-events/arch/test/test_soc/cpu/uncore.json index 788766f45dbc..73089c682f80 100644 --- a/tools/perf/pmu-events/arch/test/test_soc/cpu/uncore.json +++ b/tools/perf/pmu-events/arch/test/test_soc/cpu/uncore.json @@ -38,5 +38,5 @@ "BriefDescription": "Total cache hits", "PublicDescription": "Total cache hits", "Unit": "imc" - }, + } ] diff --git a/tools/perf/pmu-events/arch/x86/icelakex/icx-metrics.json b/tools/perf/pmu-events/arch/x86/icelakex/icx-metrics.json index 57ddbb9f9b31..14b9a8ab15b9 100644 --- a/tools/perf/pmu-events/arch/x86/icelakex/icx-metrics.json +++ b/tools/perf/pmu-events/arch/x86/icelakex/icx-metrics.json @@ -311,5 +311,5 @@ "MetricExpr": "(cstate_pkg@c6\\-residency@ / msr@tsc@) * 100", "MetricGroup": "Power", "MetricName": "C6_Pkg_Residency" - }, + } ] -- 2.28.0 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [PATCH 1/3] perf vendor-events: Fix all remaining invalid JSON files 2021-10-07 11:05 ` James Clark @ 2021-10-07 17:22 ` John Garry -1 siblings, 0 replies; 36+ messages in thread From: John Garry @ 2021-10-07 17:22 UTC (permalink / raw) To: James Clark, acme, ak, linux-perf-users Cc: Nick.Forrington, Andrew.Kilroy, Will Deacon, Mathieu Poirier, Leo Yan, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim, linux-arm-kernel, linux-kernel On 07/10/2021 12:05, James Clark wrote: > Remove trailing commas. A later commit will make the parser more strict > and these will not be valid anymore. > > Signed-off-by: James Clark<james.clark@arm.com> > --- > .../arch/arm64/ampere/emag/bus.json | 2 +- > .../arch/arm64/ampere/emag/cache.json | 20 +++++++++---------- > .../arch/arm64/ampere/emag/clock.json | 2 +- > .../arch/arm64/ampere/emag/exception.json | 4 ++-- > .../arch/arm64/ampere/emag/instruction.json | 10 +++++----- > .../arch/arm64/ampere/emag/memory.json | 4 ++-- > .../arch/arm64/hisilicon/hip08/metrics.json | 2 +- > .../pmu-events/arch/nds32/n13/atcpmu.json | 2 +- > .../pmu-events/arch/s390/cf_z10/basic.json | 2 +- > .../pmu-events/arch/s390/cf_z10/crypto.json | 2 +- > .../pmu-events/arch/s390/cf_z10/extended.json | 2 +- > .../pmu-events/arch/s390/cf_z13/basic.json | 2 +- > .../pmu-events/arch/s390/cf_z13/crypto.json | 2 +- > .../pmu-events/arch/s390/cf_z13/extended.json | 2 +- > .../pmu-events/arch/s390/cf_z14/basic.json | 2 +- > .../pmu-events/arch/s390/cf_z14/crypto.json | 2 +- > .../pmu-events/arch/s390/cf_z14/extended.json | 2 +- > .../pmu-events/arch/s390/cf_z15/basic.json | 2 +- > .../pmu-events/arch/s390/cf_z15/crypto.json | 2 +- > .../pmu-events/arch/s390/cf_z15/crypto6.json | 2 +- > .../pmu-events/arch/s390/cf_z15/extended.json | 2 +- > .../pmu-events/arch/s390/cf_z196/basic.json | 2 +- > .../pmu-events/arch/s390/cf_z196/crypto.json | 2 +- > .../arch/s390/cf_z196/extended.json | 2 +- > .../pmu-events/arch/s390/cf_zec12/basic.json | 2 +- > .../pmu-events/arch/s390/cf_zec12/crypto.json | 2 +- > .../arch/s390/cf_zec12/extended.json | 2 +- > .../arch/test/test_soc/cpu/uncore.json | 2 +- > .../arch/x86/icelakex/icx-metrics.json | 2 +- This seems fine. But, as mentioned earlier, I do worry that some of these JSONs are copied from some downstream repositories, and now they will go out of sync. That could cause problems, so need to check with respective owners. Apart from that caveat, it seems ok: Reviewed-by: John Garry <john.garry@huawei.com> ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 1/3] perf vendor-events: Fix all remaining invalid JSON files @ 2021-10-07 17:22 ` John Garry 0 siblings, 0 replies; 36+ messages in thread From: John Garry @ 2021-10-07 17:22 UTC (permalink / raw) To: James Clark, acme, ak, linux-perf-users Cc: Nick.Forrington, Andrew.Kilroy, Will Deacon, Mathieu Poirier, Leo Yan, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim, linux-arm-kernel, linux-kernel On 07/10/2021 12:05, James Clark wrote: > Remove trailing commas. A later commit will make the parser more strict > and these will not be valid anymore. > > Signed-off-by: James Clark<james.clark@arm.com> > --- > .../arch/arm64/ampere/emag/bus.json | 2 +- > .../arch/arm64/ampere/emag/cache.json | 20 +++++++++---------- > .../arch/arm64/ampere/emag/clock.json | 2 +- > .../arch/arm64/ampere/emag/exception.json | 4 ++-- > .../arch/arm64/ampere/emag/instruction.json | 10 +++++----- > .../arch/arm64/ampere/emag/memory.json | 4 ++-- > .../arch/arm64/hisilicon/hip08/metrics.json | 2 +- > .../pmu-events/arch/nds32/n13/atcpmu.json | 2 +- > .../pmu-events/arch/s390/cf_z10/basic.json | 2 +- > .../pmu-events/arch/s390/cf_z10/crypto.json | 2 +- > .../pmu-events/arch/s390/cf_z10/extended.json | 2 +- > .../pmu-events/arch/s390/cf_z13/basic.json | 2 +- > .../pmu-events/arch/s390/cf_z13/crypto.json | 2 +- > .../pmu-events/arch/s390/cf_z13/extended.json | 2 +- > .../pmu-events/arch/s390/cf_z14/basic.json | 2 +- > .../pmu-events/arch/s390/cf_z14/crypto.json | 2 +- > .../pmu-events/arch/s390/cf_z14/extended.json | 2 +- > .../pmu-events/arch/s390/cf_z15/basic.json | 2 +- > .../pmu-events/arch/s390/cf_z15/crypto.json | 2 +- > .../pmu-events/arch/s390/cf_z15/crypto6.json | 2 +- > .../pmu-events/arch/s390/cf_z15/extended.json | 2 +- > .../pmu-events/arch/s390/cf_z196/basic.json | 2 +- > .../pmu-events/arch/s390/cf_z196/crypto.json | 2 +- > .../arch/s390/cf_z196/extended.json | 2 +- > .../pmu-events/arch/s390/cf_zec12/basic.json | 2 +- > .../pmu-events/arch/s390/cf_zec12/crypto.json | 2 +- > .../arch/s390/cf_zec12/extended.json | 2 +- > .../arch/test/test_soc/cpu/uncore.json | 2 +- > .../arch/x86/icelakex/icx-metrics.json | 2 +- This seems fine. But, as mentioned earlier, I do worry that some of these JSONs are copied from some downstream repositories, and now they will go out of sync. That could cause problems, so need to check with respective owners. Apart from that caveat, it seems ok: Reviewed-by: John Garry <john.garry@huawei.com> _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 2/3] perf tools: Make the JSON parser more conformant when in strict mode 2021-10-07 11:05 ` James Clark @ 2021-10-07 11:05 ` James Clark -1 siblings, 0 replies; 36+ messages in thread From: James Clark @ 2021-10-07 11:05 UTC (permalink / raw) To: acme, john.garry, ak, linux-perf-users Cc: Nick.Forrington, Andrew.Kilroy, James Clark, Will Deacon, Mathieu Poirier, Leo Yan, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim, linux-arm-kernel, linux-kernel Return an error when a trailing comma is found or a new item is encountered before a comma or an opening brace. This ensures that the perf json files conform more closely to the spec at https://www.json.org Signed-off-by: James Clark <james.clark@arm.com> --- tools/perf/pmu-events/jsmn.c | 42 ++++++++++++++++++++++++++++++++++-- 1 file changed, 40 insertions(+), 2 deletions(-) diff --git a/tools/perf/pmu-events/jsmn.c b/tools/perf/pmu-events/jsmn.c index 11d1fa18bfa5..8124d2d3ff0c 100644 --- a/tools/perf/pmu-events/jsmn.c +++ b/tools/perf/pmu-events/jsmn.c @@ -176,6 +176,14 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, jsmnerr_t r; int i; jsmntok_t *token; +#ifdef JSMN_STRICT + /* + * Keeps track of whether a new object/list/primitive is expected. New items are only + * allowed after an opening brace, comma or colon. A closing brace after a comma is not + * valid JSON. + */ + int expecting_item = 1; +#endif for (; parser->pos < len; parser->pos++) { char c; @@ -185,6 +193,10 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, switch (c) { case '{': case '[': +#ifdef JSMN_STRICT + if (!expecting_item) + return JSMN_ERROR_INVAL; +#endif token = jsmn_alloc_token(parser, tokens, num_tokens); if (token == NULL) return JSMN_ERROR_NOMEM; @@ -196,6 +208,10 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, break; case '}': case ']': +#ifdef JSMN_STRICT + if (expecting_item) + return JSMN_ERROR_INVAL; +#endif type = (c == '}' ? JSMN_OBJECT : JSMN_ARRAY); for (i = parser->toknext - 1; i >= 0; i--) { token = &tokens[i]; @@ -219,6 +235,11 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, } break; case '\"': +#ifdef JSMN_STRICT + if (!expecting_item) + return JSMN_ERROR_INVAL; + expecting_item = 0; +#endif r = jsmn_parse_string(parser, js, len, tokens, num_tokens); if (r < 0) @@ -229,11 +250,15 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, case '\t': case '\r': case '\n': - case ':': - case ',': case ' ': break; #ifdef JSMN_STRICT + case ':': + case ',': + if (expecting_item) + return JSMN_ERROR_INVAL; + expecting_item = 1; + break; /* * In strict mode primitives are: * numbers and booleans. @@ -253,6 +278,9 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, case 'f': case 'n': #else + case ':': + case ',': + break; /* * In non-strict mode every unquoted value * is a primitive. @@ -260,6 +288,12 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, /*FALL THROUGH */ default: #endif + +#ifdef JSMN_STRICT + if (!expecting_item) + return JSMN_ERROR_INVAL; + expecting_item = 0; +#endif r = jsmn_parse_primitive(parser, js, len, tokens, num_tokens); if (r < 0) @@ -282,7 +316,11 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, return JSMN_ERROR_PART; } +#ifdef JSMN_STRICT + return expecting_item ? JSMN_ERROR_INVAL : JSMN_SUCCESS; +#else return JSMN_SUCCESS; +#endif } /* -- 2.28.0 ^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH 2/3] perf tools: Make the JSON parser more conformant when in strict mode @ 2021-10-07 11:05 ` James Clark 0 siblings, 0 replies; 36+ messages in thread From: James Clark @ 2021-10-07 11:05 UTC (permalink / raw) To: acme, john.garry, ak, linux-perf-users Cc: Nick.Forrington, Andrew.Kilroy, James Clark, Will Deacon, Mathieu Poirier, Leo Yan, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim, linux-arm-kernel, linux-kernel Return an error when a trailing comma is found or a new item is encountered before a comma or an opening brace. This ensures that the perf json files conform more closely to the spec at https://www.json.org Signed-off-by: James Clark <james.clark@arm.com> --- tools/perf/pmu-events/jsmn.c | 42 ++++++++++++++++++++++++++++++++++-- 1 file changed, 40 insertions(+), 2 deletions(-) diff --git a/tools/perf/pmu-events/jsmn.c b/tools/perf/pmu-events/jsmn.c index 11d1fa18bfa5..8124d2d3ff0c 100644 --- a/tools/perf/pmu-events/jsmn.c +++ b/tools/perf/pmu-events/jsmn.c @@ -176,6 +176,14 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, jsmnerr_t r; int i; jsmntok_t *token; +#ifdef JSMN_STRICT + /* + * Keeps track of whether a new object/list/primitive is expected. New items are only + * allowed after an opening brace, comma or colon. A closing brace after a comma is not + * valid JSON. + */ + int expecting_item = 1; +#endif for (; parser->pos < len; parser->pos++) { char c; @@ -185,6 +193,10 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, switch (c) { case '{': case '[': +#ifdef JSMN_STRICT + if (!expecting_item) + return JSMN_ERROR_INVAL; +#endif token = jsmn_alloc_token(parser, tokens, num_tokens); if (token == NULL) return JSMN_ERROR_NOMEM; @@ -196,6 +208,10 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, break; case '}': case ']': +#ifdef JSMN_STRICT + if (expecting_item) + return JSMN_ERROR_INVAL; +#endif type = (c == '}' ? JSMN_OBJECT : JSMN_ARRAY); for (i = parser->toknext - 1; i >= 0; i--) { token = &tokens[i]; @@ -219,6 +235,11 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, } break; case '\"': +#ifdef JSMN_STRICT + if (!expecting_item) + return JSMN_ERROR_INVAL; + expecting_item = 0; +#endif r = jsmn_parse_string(parser, js, len, tokens, num_tokens); if (r < 0) @@ -229,11 +250,15 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, case '\t': case '\r': case '\n': - case ':': - case ',': case ' ': break; #ifdef JSMN_STRICT + case ':': + case ',': + if (expecting_item) + return JSMN_ERROR_INVAL; + expecting_item = 1; + break; /* * In strict mode primitives are: * numbers and booleans. @@ -253,6 +278,9 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, case 'f': case 'n': #else + case ':': + case ',': + break; /* * In non-strict mode every unquoted value * is a primitive. @@ -260,6 +288,12 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, /*FALL THROUGH */ default: #endif + +#ifdef JSMN_STRICT + if (!expecting_item) + return JSMN_ERROR_INVAL; + expecting_item = 0; +#endif r = jsmn_parse_primitive(parser, js, len, tokens, num_tokens); if (r < 0) @@ -282,7 +316,11 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, return JSMN_ERROR_PART; } +#ifdef JSMN_STRICT + return expecting_item ? JSMN_ERROR_INVAL : JSMN_SUCCESS; +#else return JSMN_SUCCESS; +#endif } /* -- 2.28.0 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [PATCH 2/3] perf tools: Make the JSON parser more conformant when in strict mode 2021-10-07 11:05 ` James Clark @ 2021-10-07 17:52 ` Jiri Olsa -1 siblings, 0 replies; 36+ messages in thread From: Jiri Olsa @ 2021-10-07 17:52 UTC (permalink / raw) To: James Clark Cc: acme, john.garry, ak, linux-perf-users, Nick.Forrington, Andrew.Kilroy, Will Deacon, Mathieu Poirier, Leo Yan, Mark Rutland, Alexander Shishkin, Namhyung Kim, linux-arm-kernel, linux-kernel On Thu, Oct 07, 2021 at 12:05:41PM +0100, James Clark wrote: > Return an error when a trailing comma is found or a new item is > encountered before a comma or an opening brace. This ensures that the > perf json files conform more closely to the spec at https://www.json.org > > Signed-off-by: James Clark <james.clark@arm.com> > --- > tools/perf/pmu-events/jsmn.c | 42 ++++++++++++++++++++++++++++++++++-- > 1 file changed, 40 insertions(+), 2 deletions(-) > > diff --git a/tools/perf/pmu-events/jsmn.c b/tools/perf/pmu-events/jsmn.c > index 11d1fa18bfa5..8124d2d3ff0c 100644 > --- a/tools/perf/pmu-events/jsmn.c > +++ b/tools/perf/pmu-events/jsmn.c > @@ -176,6 +176,14 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, > jsmnerr_t r; > int i; > jsmntok_t *token; > +#ifdef JSMN_STRICT I might have missed some discussion on this, but do we need the JSMN_STRICT define, if you enable it in the next patch? why can't we be more strict by default.. do you plan to disable it in future? thanks, jirka > + /* > + * Keeps track of whether a new object/list/primitive is expected. New items are only > + * allowed after an opening brace, comma or colon. A closing brace after a comma is not > + * valid JSON. > + */ > + int expecting_item = 1; > +#endif > > for (; parser->pos < len; parser->pos++) { > char c; > @@ -185,6 +193,10 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, > switch (c) { > case '{': > case '[': > +#ifdef JSMN_STRICT > + if (!expecting_item) > + return JSMN_ERROR_INVAL; > +#endif > token = jsmn_alloc_token(parser, tokens, num_tokens); > if (token == NULL) > return JSMN_ERROR_NOMEM; > @@ -196,6 +208,10 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, > break; > case '}': > case ']': > +#ifdef JSMN_STRICT > + if (expecting_item) > + return JSMN_ERROR_INVAL; > +#endif > type = (c == '}' ? JSMN_OBJECT : JSMN_ARRAY); > for (i = parser->toknext - 1; i >= 0; i--) { > token = &tokens[i]; > @@ -219,6 +235,11 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, > } > break; > case '\"': > +#ifdef JSMN_STRICT > + if (!expecting_item) > + return JSMN_ERROR_INVAL; > + expecting_item = 0; > +#endif > r = jsmn_parse_string(parser, js, len, tokens, > num_tokens); > if (r < 0) > @@ -229,11 +250,15 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, > case '\t': > case '\r': > case '\n': > - case ':': > - case ',': > case ' ': > break; > #ifdef JSMN_STRICT > + case ':': > + case ',': > + if (expecting_item) > + return JSMN_ERROR_INVAL; > + expecting_item = 1; > + break; > /* > * In strict mode primitives are: > * numbers and booleans. > @@ -253,6 +278,9 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, > case 'f': > case 'n': > #else > + case ':': > + case ',': > + break; > /* > * In non-strict mode every unquoted value > * is a primitive. > @@ -260,6 +288,12 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, > /*FALL THROUGH */ > default: > #endif > + > +#ifdef JSMN_STRICT > + if (!expecting_item) > + return JSMN_ERROR_INVAL; > + expecting_item = 0; > +#endif > r = jsmn_parse_primitive(parser, js, len, tokens, > num_tokens); > if (r < 0) > @@ -282,7 +316,11 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, > return JSMN_ERROR_PART; > } > > +#ifdef JSMN_STRICT > + return expecting_item ? JSMN_ERROR_INVAL : JSMN_SUCCESS; > +#else > return JSMN_SUCCESS; > +#endif > } > > /* > -- > 2.28.0 > ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 2/3] perf tools: Make the JSON parser more conformant when in strict mode @ 2021-10-07 17:52 ` Jiri Olsa 0 siblings, 0 replies; 36+ messages in thread From: Jiri Olsa @ 2021-10-07 17:52 UTC (permalink / raw) To: James Clark Cc: acme, john.garry, ak, linux-perf-users, Nick.Forrington, Andrew.Kilroy, Will Deacon, Mathieu Poirier, Leo Yan, Mark Rutland, Alexander Shishkin, Namhyung Kim, linux-arm-kernel, linux-kernel On Thu, Oct 07, 2021 at 12:05:41PM +0100, James Clark wrote: > Return an error when a trailing comma is found or a new item is > encountered before a comma or an opening brace. This ensures that the > perf json files conform more closely to the spec at https://www.json.org > > Signed-off-by: James Clark <james.clark@arm.com> > --- > tools/perf/pmu-events/jsmn.c | 42 ++++++++++++++++++++++++++++++++++-- > 1 file changed, 40 insertions(+), 2 deletions(-) > > diff --git a/tools/perf/pmu-events/jsmn.c b/tools/perf/pmu-events/jsmn.c > index 11d1fa18bfa5..8124d2d3ff0c 100644 > --- a/tools/perf/pmu-events/jsmn.c > +++ b/tools/perf/pmu-events/jsmn.c > @@ -176,6 +176,14 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, > jsmnerr_t r; > int i; > jsmntok_t *token; > +#ifdef JSMN_STRICT I might have missed some discussion on this, but do we need the JSMN_STRICT define, if you enable it in the next patch? why can't we be more strict by default.. do you plan to disable it in future? thanks, jirka > + /* > + * Keeps track of whether a new object/list/primitive is expected. New items are only > + * allowed after an opening brace, comma or colon. A closing brace after a comma is not > + * valid JSON. > + */ > + int expecting_item = 1; > +#endif > > for (; parser->pos < len; parser->pos++) { > char c; > @@ -185,6 +193,10 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, > switch (c) { > case '{': > case '[': > +#ifdef JSMN_STRICT > + if (!expecting_item) > + return JSMN_ERROR_INVAL; > +#endif > token = jsmn_alloc_token(parser, tokens, num_tokens); > if (token == NULL) > return JSMN_ERROR_NOMEM; > @@ -196,6 +208,10 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, > break; > case '}': > case ']': > +#ifdef JSMN_STRICT > + if (expecting_item) > + return JSMN_ERROR_INVAL; > +#endif > type = (c == '}' ? JSMN_OBJECT : JSMN_ARRAY); > for (i = parser->toknext - 1; i >= 0; i--) { > token = &tokens[i]; > @@ -219,6 +235,11 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, > } > break; > case '\"': > +#ifdef JSMN_STRICT > + if (!expecting_item) > + return JSMN_ERROR_INVAL; > + expecting_item = 0; > +#endif > r = jsmn_parse_string(parser, js, len, tokens, > num_tokens); > if (r < 0) > @@ -229,11 +250,15 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, > case '\t': > case '\r': > case '\n': > - case ':': > - case ',': > case ' ': > break; > #ifdef JSMN_STRICT > + case ':': > + case ',': > + if (expecting_item) > + return JSMN_ERROR_INVAL; > + expecting_item = 1; > + break; > /* > * In strict mode primitives are: > * numbers and booleans. > @@ -253,6 +278,9 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, > case 'f': > case 'n': > #else > + case ':': > + case ',': > + break; > /* > * In non-strict mode every unquoted value > * is a primitive. > @@ -260,6 +288,12 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, > /*FALL THROUGH */ > default: > #endif > + > +#ifdef JSMN_STRICT > + if (!expecting_item) > + return JSMN_ERROR_INVAL; > + expecting_item = 0; > +#endif > r = jsmn_parse_primitive(parser, js, len, tokens, > num_tokens); > if (r < 0) > @@ -282,7 +316,11 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, > return JSMN_ERROR_PART; > } > > +#ifdef JSMN_STRICT > + return expecting_item ? JSMN_ERROR_INVAL : JSMN_SUCCESS; > +#else > return JSMN_SUCCESS; > +#endif > } > > /* > -- > 2.28.0 > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 2/3] perf tools: Make the JSON parser more conformant when in strict mode 2021-10-07 17:52 ` Jiri Olsa @ 2021-10-08 10:08 ` James Clark -1 siblings, 0 replies; 36+ messages in thread From: James Clark @ 2021-10-08 10:08 UTC (permalink / raw) To: Jiri Olsa Cc: acme, john.garry, ak, linux-perf-users, Nick.Forrington, Andrew.Kilroy, Will Deacon, Mathieu Poirier, Leo Yan, Mark Rutland, Alexander Shishkin, Namhyung Kim, linux-arm-kernel, linux-kernel On 07/10/2021 18:52, Jiri Olsa wrote: > On Thu, Oct 07, 2021 at 12:05:41PM +0100, James Clark wrote: >> Return an error when a trailing comma is found or a new item is >> encountered before a comma or an opening brace. This ensures that the >> perf json files conform more closely to the spec at https://www.json.org >> >> Signed-off-by: James Clark <james.clark@arm.com> >> --- >> tools/perf/pmu-events/jsmn.c | 42 ++++++++++++++++++++++++++++++++++-- >> 1 file changed, 40 insertions(+), 2 deletions(-) >> >> diff --git a/tools/perf/pmu-events/jsmn.c b/tools/perf/pmu-events/jsmn.c >> index 11d1fa18bfa5..8124d2d3ff0c 100644 >> --- a/tools/perf/pmu-events/jsmn.c >> +++ b/tools/perf/pmu-events/jsmn.c >> @@ -176,6 +176,14 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, >> jsmnerr_t r; >> int i; >> jsmntok_t *token; >> +#ifdef JSMN_STRICT > > I might have missed some discussion on this, but do we need the > JSMN_STRICT define, if you enable it in the next patch? > why can't we be more strict by default.. do you plan to disable > it in future? I didn't plan on disabling it, I was just trying to keep to the existing style of the jsmn project. I could have added the trailing comma detection by default and not inside any #ifdef JSMN_STRICT blocks, but I would like to enable JSMN_STRICT anyway, because it enables some additional built in checking that was already there. So I thought it made sense to put my new strict stuff inside the existing strict option. One option would be to remove all (including the existing) #ifdef JSMN_STRICT blocks and have everything strict by default. But it would be a further deviation from jsmn. Thanks James > > thanks, > jirka > >> + /* >> + * Keeps track of whether a new object/list/primitive is expected. New items are only >> + * allowed after an opening brace, comma or colon. A closing brace after a comma is not >> + * valid JSON. >> + */ >> + int expecting_item = 1; >> +#endif >> >> for (; parser->pos < len; parser->pos++) { >> char c; >> @@ -185,6 +193,10 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, >> switch (c) { >> case '{': >> case '[': >> +#ifdef JSMN_STRICT >> + if (!expecting_item) >> + return JSMN_ERROR_INVAL; >> +#endif >> token = jsmn_alloc_token(parser, tokens, num_tokens); >> if (token == NULL) >> return JSMN_ERROR_NOMEM; >> @@ -196,6 +208,10 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, >> break; >> case '}': >> case ']': >> +#ifdef JSMN_STRICT >> + if (expecting_item) >> + return JSMN_ERROR_INVAL; >> +#endif >> type = (c == '}' ? JSMN_OBJECT : JSMN_ARRAY); >> for (i = parser->toknext - 1; i >= 0; i--) { >> token = &tokens[i]; >> @@ -219,6 +235,11 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, >> } >> break; >> case '\"': >> +#ifdef JSMN_STRICT >> + if (!expecting_item) >> + return JSMN_ERROR_INVAL; >> + expecting_item = 0; >> +#endif >> r = jsmn_parse_string(parser, js, len, tokens, >> num_tokens); >> if (r < 0) >> @@ -229,11 +250,15 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, >> case '\t': >> case '\r': >> case '\n': >> - case ':': >> - case ',': >> case ' ': >> break; >> #ifdef JSMN_STRICT >> + case ':': >> + case ',': >> + if (expecting_item) >> + return JSMN_ERROR_INVAL; >> + expecting_item = 1; >> + break; >> /* >> * In strict mode primitives are: >> * numbers and booleans. >> @@ -253,6 +278,9 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, >> case 'f': >> case 'n': >> #else >> + case ':': >> + case ',': >> + break; >> /* >> * In non-strict mode every unquoted value >> * is a primitive. >> @@ -260,6 +288,12 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, >> /*FALL THROUGH */ >> default: >> #endif >> + >> +#ifdef JSMN_STRICT >> + if (!expecting_item) >> + return JSMN_ERROR_INVAL; >> + expecting_item = 0; >> +#endif >> r = jsmn_parse_primitive(parser, js, len, tokens, >> num_tokens); >> if (r < 0) >> @@ -282,7 +316,11 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, >> return JSMN_ERROR_PART; >> } >> >> +#ifdef JSMN_STRICT >> + return expecting_item ? JSMN_ERROR_INVAL : JSMN_SUCCESS; >> +#else >> return JSMN_SUCCESS; >> +#endif >> } >> >> /* >> -- >> 2.28.0 >> > ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 2/3] perf tools: Make the JSON parser more conformant when in strict mode @ 2021-10-08 10:08 ` James Clark 0 siblings, 0 replies; 36+ messages in thread From: James Clark @ 2021-10-08 10:08 UTC (permalink / raw) To: Jiri Olsa Cc: acme, john.garry, ak, linux-perf-users, Nick.Forrington, Andrew.Kilroy, Will Deacon, Mathieu Poirier, Leo Yan, Mark Rutland, Alexander Shishkin, Namhyung Kim, linux-arm-kernel, linux-kernel On 07/10/2021 18:52, Jiri Olsa wrote: > On Thu, Oct 07, 2021 at 12:05:41PM +0100, James Clark wrote: >> Return an error when a trailing comma is found or a new item is >> encountered before a comma or an opening brace. This ensures that the >> perf json files conform more closely to the spec at https://www.json.org >> >> Signed-off-by: James Clark <james.clark@arm.com> >> --- >> tools/perf/pmu-events/jsmn.c | 42 ++++++++++++++++++++++++++++++++++-- >> 1 file changed, 40 insertions(+), 2 deletions(-) >> >> diff --git a/tools/perf/pmu-events/jsmn.c b/tools/perf/pmu-events/jsmn.c >> index 11d1fa18bfa5..8124d2d3ff0c 100644 >> --- a/tools/perf/pmu-events/jsmn.c >> +++ b/tools/perf/pmu-events/jsmn.c >> @@ -176,6 +176,14 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, >> jsmnerr_t r; >> int i; >> jsmntok_t *token; >> +#ifdef JSMN_STRICT > > I might have missed some discussion on this, but do we need the > JSMN_STRICT define, if you enable it in the next patch? > why can't we be more strict by default.. do you plan to disable > it in future? I didn't plan on disabling it, I was just trying to keep to the existing style of the jsmn project. I could have added the trailing comma detection by default and not inside any #ifdef JSMN_STRICT blocks, but I would like to enable JSMN_STRICT anyway, because it enables some additional built in checking that was already there. So I thought it made sense to put my new strict stuff inside the existing strict option. One option would be to remove all (including the existing) #ifdef JSMN_STRICT blocks and have everything strict by default. But it would be a further deviation from jsmn. Thanks James > > thanks, > jirka > >> + /* >> + * Keeps track of whether a new object/list/primitive is expected. New items are only >> + * allowed after an opening brace, comma or colon. A closing brace after a comma is not >> + * valid JSON. >> + */ >> + int expecting_item = 1; >> +#endif >> >> for (; parser->pos < len; parser->pos++) { >> char c; >> @@ -185,6 +193,10 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, >> switch (c) { >> case '{': >> case '[': >> +#ifdef JSMN_STRICT >> + if (!expecting_item) >> + return JSMN_ERROR_INVAL; >> +#endif >> token = jsmn_alloc_token(parser, tokens, num_tokens); >> if (token == NULL) >> return JSMN_ERROR_NOMEM; >> @@ -196,6 +208,10 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, >> break; >> case '}': >> case ']': >> +#ifdef JSMN_STRICT >> + if (expecting_item) >> + return JSMN_ERROR_INVAL; >> +#endif >> type = (c == '}' ? JSMN_OBJECT : JSMN_ARRAY); >> for (i = parser->toknext - 1; i >= 0; i--) { >> token = &tokens[i]; >> @@ -219,6 +235,11 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, >> } >> break; >> case '\"': >> +#ifdef JSMN_STRICT >> + if (!expecting_item) >> + return JSMN_ERROR_INVAL; >> + expecting_item = 0; >> +#endif >> r = jsmn_parse_string(parser, js, len, tokens, >> num_tokens); >> if (r < 0) >> @@ -229,11 +250,15 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, >> case '\t': >> case '\r': >> case '\n': >> - case ':': >> - case ',': >> case ' ': >> break; >> #ifdef JSMN_STRICT >> + case ':': >> + case ',': >> + if (expecting_item) >> + return JSMN_ERROR_INVAL; >> + expecting_item = 1; >> + break; >> /* >> * In strict mode primitives are: >> * numbers and booleans. >> @@ -253,6 +278,9 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, >> case 'f': >> case 'n': >> #else >> + case ':': >> + case ',': >> + break; >> /* >> * In non-strict mode every unquoted value >> * is a primitive. >> @@ -260,6 +288,12 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, >> /*FALL THROUGH */ >> default: >> #endif >> + >> +#ifdef JSMN_STRICT >> + if (!expecting_item) >> + return JSMN_ERROR_INVAL; >> + expecting_item = 0; >> +#endif >> r = jsmn_parse_primitive(parser, js, len, tokens, >> num_tokens); >> if (r < 0) >> @@ -282,7 +316,11 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, >> return JSMN_ERROR_PART; >> } >> >> +#ifdef JSMN_STRICT >> + return expecting_item ? JSMN_ERROR_INVAL : JSMN_SUCCESS; >> +#else >> return JSMN_SUCCESS; >> +#endif >> } >> >> /* >> -- >> 2.28.0 >> > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 2/3] perf tools: Make the JSON parser more conformant when in strict mode 2021-10-08 10:08 ` James Clark @ 2021-10-08 13:12 ` Jiri Olsa -1 siblings, 0 replies; 36+ messages in thread From: Jiri Olsa @ 2021-10-08 13:12 UTC (permalink / raw) To: James Clark Cc: acme, john.garry, ak, linux-perf-users, Nick.Forrington, Andrew.Kilroy, Will Deacon, Mathieu Poirier, Leo Yan, Mark Rutland, Alexander Shishkin, Namhyung Kim, linux-arm-kernel, linux-kernel On Fri, Oct 08, 2021 at 11:08:25AM +0100, James Clark wrote: > > > On 07/10/2021 18:52, Jiri Olsa wrote: > > On Thu, Oct 07, 2021 at 12:05:41PM +0100, James Clark wrote: > >> Return an error when a trailing comma is found or a new item is > >> encountered before a comma or an opening brace. This ensures that the > >> perf json files conform more closely to the spec at https://www.json.org > >> > >> Signed-off-by: James Clark <james.clark@arm.com> > >> --- > >> tools/perf/pmu-events/jsmn.c | 42 ++++++++++++++++++++++++++++++++++-- > >> 1 file changed, 40 insertions(+), 2 deletions(-) > >> > >> diff --git a/tools/perf/pmu-events/jsmn.c b/tools/perf/pmu-events/jsmn.c > >> index 11d1fa18bfa5..8124d2d3ff0c 100644 > >> --- a/tools/perf/pmu-events/jsmn.c > >> +++ b/tools/perf/pmu-events/jsmn.c > >> @@ -176,6 +176,14 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, > >> jsmnerr_t r; > >> int i; > >> jsmntok_t *token; > >> +#ifdef JSMN_STRICT > > > > I might have missed some discussion on this, but do we need the > > JSMN_STRICT define, if you enable it in the next patch? > > why can't we be more strict by default.. do you plan to disable > > it in future? > > I didn't plan on disabling it, I was just trying to keep to the existing style of the > jsmn project. > > I could have added the trailing comma detection by default and not inside any > #ifdef JSMN_STRICT blocks, but I would like to enable JSMN_STRICT anyway, because it > enables some additional built in checking that was already there. So I thought it > made sense to put my new strict stuff inside the existing strict option. > > One option would be to remove all (including the existing) #ifdef JSMN_STRICT blocks > and have everything strict by default. But it would be a further deviation from jsmn. ok, I think it makes sense to have JSMN_STRICT then.. thanks for explanation Acked-by: Jiri Olsa <jolsa@redhat.com> jirka > > Thanks > James > > > > > thanks, > > jirka > > > >> + /* > >> + * Keeps track of whether a new object/list/primitive is expected. New items are only > >> + * allowed after an opening brace, comma or colon. A closing brace after a comma is not > >> + * valid JSON. > >> + */ > >> + int expecting_item = 1; > >> +#endif > >> > >> for (; parser->pos < len; parser->pos++) { > >> char c; > >> @@ -185,6 +193,10 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, > >> switch (c) { > >> case '{': > >> case '[': > >> +#ifdef JSMN_STRICT > >> + if (!expecting_item) > >> + return JSMN_ERROR_INVAL; > >> +#endif > >> token = jsmn_alloc_token(parser, tokens, num_tokens); > >> if (token == NULL) > >> return JSMN_ERROR_NOMEM; > >> @@ -196,6 +208,10 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, > >> break; > >> case '}': > >> case ']': > >> +#ifdef JSMN_STRICT > >> + if (expecting_item) > >> + return JSMN_ERROR_INVAL; > >> +#endif > >> type = (c == '}' ? JSMN_OBJECT : JSMN_ARRAY); > >> for (i = parser->toknext - 1; i >= 0; i--) { > >> token = &tokens[i]; > >> @@ -219,6 +235,11 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, > >> } > >> break; > >> case '\"': > >> +#ifdef JSMN_STRICT > >> + if (!expecting_item) > >> + return JSMN_ERROR_INVAL; > >> + expecting_item = 0; > >> +#endif > >> r = jsmn_parse_string(parser, js, len, tokens, > >> num_tokens); > >> if (r < 0) > >> @@ -229,11 +250,15 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, > >> case '\t': > >> case '\r': > >> case '\n': > >> - case ':': > >> - case ',': > >> case ' ': > >> break; > >> #ifdef JSMN_STRICT > >> + case ':': > >> + case ',': > >> + if (expecting_item) > >> + return JSMN_ERROR_INVAL; > >> + expecting_item = 1; > >> + break; > >> /* > >> * In strict mode primitives are: > >> * numbers and booleans. > >> @@ -253,6 +278,9 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, > >> case 'f': > >> case 'n': > >> #else > >> + case ':': > >> + case ',': > >> + break; > >> /* > >> * In non-strict mode every unquoted value > >> * is a primitive. > >> @@ -260,6 +288,12 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, > >> /*FALL THROUGH */ > >> default: > >> #endif > >> + > >> +#ifdef JSMN_STRICT > >> + if (!expecting_item) > >> + return JSMN_ERROR_INVAL; > >> + expecting_item = 0; > >> +#endif > >> r = jsmn_parse_primitive(parser, js, len, tokens, > >> num_tokens); > >> if (r < 0) > >> @@ -282,7 +316,11 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, > >> return JSMN_ERROR_PART; > >> } > >> > >> +#ifdef JSMN_STRICT > >> + return expecting_item ? JSMN_ERROR_INVAL : JSMN_SUCCESS; > >> +#else > >> return JSMN_SUCCESS; > >> +#endif > >> } > >> > >> /* > >> -- > >> 2.28.0 > >> > > > ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 2/3] perf tools: Make the JSON parser more conformant when in strict mode @ 2021-10-08 13:12 ` Jiri Olsa 0 siblings, 0 replies; 36+ messages in thread From: Jiri Olsa @ 2021-10-08 13:12 UTC (permalink / raw) To: James Clark Cc: acme, john.garry, ak, linux-perf-users, Nick.Forrington, Andrew.Kilroy, Will Deacon, Mathieu Poirier, Leo Yan, Mark Rutland, Alexander Shishkin, Namhyung Kim, linux-arm-kernel, linux-kernel On Fri, Oct 08, 2021 at 11:08:25AM +0100, James Clark wrote: > > > On 07/10/2021 18:52, Jiri Olsa wrote: > > On Thu, Oct 07, 2021 at 12:05:41PM +0100, James Clark wrote: > >> Return an error when a trailing comma is found or a new item is > >> encountered before a comma or an opening brace. This ensures that the > >> perf json files conform more closely to the spec at https://www.json.org > >> > >> Signed-off-by: James Clark <james.clark@arm.com> > >> --- > >> tools/perf/pmu-events/jsmn.c | 42 ++++++++++++++++++++++++++++++++++-- > >> 1 file changed, 40 insertions(+), 2 deletions(-) > >> > >> diff --git a/tools/perf/pmu-events/jsmn.c b/tools/perf/pmu-events/jsmn.c > >> index 11d1fa18bfa5..8124d2d3ff0c 100644 > >> --- a/tools/perf/pmu-events/jsmn.c > >> +++ b/tools/perf/pmu-events/jsmn.c > >> @@ -176,6 +176,14 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, > >> jsmnerr_t r; > >> int i; > >> jsmntok_t *token; > >> +#ifdef JSMN_STRICT > > > > I might have missed some discussion on this, but do we need the > > JSMN_STRICT define, if you enable it in the next patch? > > why can't we be more strict by default.. do you plan to disable > > it in future? > > I didn't plan on disabling it, I was just trying to keep to the existing style of the > jsmn project. > > I could have added the trailing comma detection by default and not inside any > #ifdef JSMN_STRICT blocks, but I would like to enable JSMN_STRICT anyway, because it > enables some additional built in checking that was already there. So I thought it > made sense to put my new strict stuff inside the existing strict option. > > One option would be to remove all (including the existing) #ifdef JSMN_STRICT blocks > and have everything strict by default. But it would be a further deviation from jsmn. ok, I think it makes sense to have JSMN_STRICT then.. thanks for explanation Acked-by: Jiri Olsa <jolsa@redhat.com> jirka > > Thanks > James > > > > > thanks, > > jirka > > > >> + /* > >> + * Keeps track of whether a new object/list/primitive is expected. New items are only > >> + * allowed after an opening brace, comma or colon. A closing brace after a comma is not > >> + * valid JSON. > >> + */ > >> + int expecting_item = 1; > >> +#endif > >> > >> for (; parser->pos < len; parser->pos++) { > >> char c; > >> @@ -185,6 +193,10 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, > >> switch (c) { > >> case '{': > >> case '[': > >> +#ifdef JSMN_STRICT > >> + if (!expecting_item) > >> + return JSMN_ERROR_INVAL; > >> +#endif > >> token = jsmn_alloc_token(parser, tokens, num_tokens); > >> if (token == NULL) > >> return JSMN_ERROR_NOMEM; > >> @@ -196,6 +208,10 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, > >> break; > >> case '}': > >> case ']': > >> +#ifdef JSMN_STRICT > >> + if (expecting_item) > >> + return JSMN_ERROR_INVAL; > >> +#endif > >> type = (c == '}' ? JSMN_OBJECT : JSMN_ARRAY); > >> for (i = parser->toknext - 1; i >= 0; i--) { > >> token = &tokens[i]; > >> @@ -219,6 +235,11 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, > >> } > >> break; > >> case '\"': > >> +#ifdef JSMN_STRICT > >> + if (!expecting_item) > >> + return JSMN_ERROR_INVAL; > >> + expecting_item = 0; > >> +#endif > >> r = jsmn_parse_string(parser, js, len, tokens, > >> num_tokens); > >> if (r < 0) > >> @@ -229,11 +250,15 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, > >> case '\t': > >> case '\r': > >> case '\n': > >> - case ':': > >> - case ',': > >> case ' ': > >> break; > >> #ifdef JSMN_STRICT > >> + case ':': > >> + case ',': > >> + if (expecting_item) > >> + return JSMN_ERROR_INVAL; > >> + expecting_item = 1; > >> + break; > >> /* > >> * In strict mode primitives are: > >> * numbers and booleans. > >> @@ -253,6 +278,9 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, > >> case 'f': > >> case 'n': > >> #else > >> + case ':': > >> + case ',': > >> + break; > >> /* > >> * In non-strict mode every unquoted value > >> * is a primitive. > >> @@ -260,6 +288,12 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, > >> /*FALL THROUGH */ > >> default: > >> #endif > >> + > >> +#ifdef JSMN_STRICT > >> + if (!expecting_item) > >> + return JSMN_ERROR_INVAL; > >> + expecting_item = 0; > >> +#endif > >> r = jsmn_parse_primitive(parser, js, len, tokens, > >> num_tokens); > >> if (r < 0) > >> @@ -282,7 +316,11 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, > >> return JSMN_ERROR_PART; > >> } > >> > >> +#ifdef JSMN_STRICT > >> + return expecting_item ? JSMN_ERROR_INVAL : JSMN_SUCCESS; > >> +#else > >> return JSMN_SUCCESS; > >> +#endif > >> } > >> > >> /* > >> -- > >> 2.28.0 > >> > > > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 2/3] perf tools: Make the JSON parser more conformant when in strict mode 2021-10-08 13:12 ` Jiri Olsa @ 2021-10-08 18:56 ` Arnaldo Carvalho de Melo -1 siblings, 0 replies; 36+ messages in thread From: Arnaldo Carvalho de Melo @ 2021-10-08 18:56 UTC (permalink / raw) To: Jiri Olsa Cc: James Clark, john.garry, ak, linux-perf-users, Nick.Forrington, Andrew.Kilroy, Will Deacon, Mathieu Poirier, Leo Yan, Mark Rutland, Alexander Shishkin, Namhyung Kim, linux-arm-kernel, linux-kernel Em Fri, Oct 08, 2021 at 03:12:03PM +0200, Jiri Olsa escreveu: > On Fri, Oct 08, 2021 at 11:08:25AM +0100, James Clark wrote: > > > > > > On 07/10/2021 18:52, Jiri Olsa wrote: > > > On Thu, Oct 07, 2021 at 12:05:41PM +0100, James Clark wrote: > > >> Return an error when a trailing comma is found or a new item is > > >> encountered before a comma or an opening brace. This ensures that the > > >> perf json files conform more closely to the spec at https://www.json.org > > >> > > >> Signed-off-by: James Clark <james.clark@arm.com> > > >> --- > > >> tools/perf/pmu-events/jsmn.c | 42 ++++++++++++++++++++++++++++++++++-- > > >> 1 file changed, 40 insertions(+), 2 deletions(-) > > >> > > >> diff --git a/tools/perf/pmu-events/jsmn.c b/tools/perf/pmu-events/jsmn.c > > >> index 11d1fa18bfa5..8124d2d3ff0c 100644 > > >> --- a/tools/perf/pmu-events/jsmn.c > > >> +++ b/tools/perf/pmu-events/jsmn.c > > >> @@ -176,6 +176,14 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, > > >> jsmnerr_t r; > > >> int i; > > >> jsmntok_t *token; > > >> +#ifdef JSMN_STRICT > > > > > > I might have missed some discussion on this, but do we need the > > > JSMN_STRICT define, if you enable it in the next patch? > > > why can't we be more strict by default.. do you plan to disable > > > it in future? > > > > I didn't plan on disabling it, I was just trying to keep to the existing style of the > > jsmn project. > > > > I could have added the trailing comma detection by default and not inside any > > #ifdef JSMN_STRICT blocks, but I would like to enable JSMN_STRICT anyway, because it > > enables some additional built in checking that was already there. So I thought it > > made sense to put my new strict stuff inside the existing strict option. > > > > One option would be to remove all (including the existing) #ifdef JSMN_STRICT blocks > > and have everything strict by default. But it would be a further deviation from jsmn. > > ok, I think it makes sense to have JSMN_STRICT then.. > thanks for explanation > > Acked-by: Jiri Olsa <jolsa@redhat.com> So, is this for the whole patchset? b4 picked it just for this message. - Arnaldo > jirka > > > > > Thanks > > James > > > > > > > > thanks, > > > jirka > > > > > >> + /* > > >> + * Keeps track of whether a new object/list/primitive is expected. New items are only > > >> + * allowed after an opening brace, comma or colon. A closing brace after a comma is not > > >> + * valid JSON. > > >> + */ > > >> + int expecting_item = 1; > > >> +#endif > > >> > > >> for (; parser->pos < len; parser->pos++) { > > >> char c; > > >> @@ -185,6 +193,10 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, > > >> switch (c) { > > >> case '{': > > >> case '[': > > >> +#ifdef JSMN_STRICT > > >> + if (!expecting_item) > > >> + return JSMN_ERROR_INVAL; > > >> +#endif > > >> token = jsmn_alloc_token(parser, tokens, num_tokens); > > >> if (token == NULL) > > >> return JSMN_ERROR_NOMEM; > > >> @@ -196,6 +208,10 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, > > >> break; > > >> case '}': > > >> case ']': > > >> +#ifdef JSMN_STRICT > > >> + if (expecting_item) > > >> + return JSMN_ERROR_INVAL; > > >> +#endif > > >> type = (c == '}' ? JSMN_OBJECT : JSMN_ARRAY); > > >> for (i = parser->toknext - 1; i >= 0; i--) { > > >> token = &tokens[i]; > > >> @@ -219,6 +235,11 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, > > >> } > > >> break; > > >> case '\"': > > >> +#ifdef JSMN_STRICT > > >> + if (!expecting_item) > > >> + return JSMN_ERROR_INVAL; > > >> + expecting_item = 0; > > >> +#endif > > >> r = jsmn_parse_string(parser, js, len, tokens, > > >> num_tokens); > > >> if (r < 0) > > >> @@ -229,11 +250,15 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, > > >> case '\t': > > >> case '\r': > > >> case '\n': > > >> - case ':': > > >> - case ',': > > >> case ' ': > > >> break; > > >> #ifdef JSMN_STRICT > > >> + case ':': > > >> + case ',': > > >> + if (expecting_item) > > >> + return JSMN_ERROR_INVAL; > > >> + expecting_item = 1; > > >> + break; > > >> /* > > >> * In strict mode primitives are: > > >> * numbers and booleans. > > >> @@ -253,6 +278,9 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, > > >> case 'f': > > >> case 'n': > > >> #else > > >> + case ':': > > >> + case ',': > > >> + break; > > >> /* > > >> * In non-strict mode every unquoted value > > >> * is a primitive. > > >> @@ -260,6 +288,12 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, > > >> /*FALL THROUGH */ > > >> default: > > >> #endif > > >> + > > >> +#ifdef JSMN_STRICT > > >> + if (!expecting_item) > > >> + return JSMN_ERROR_INVAL; > > >> + expecting_item = 0; > > >> +#endif > > >> r = jsmn_parse_primitive(parser, js, len, tokens, > > >> num_tokens); > > >> if (r < 0) > > >> @@ -282,7 +316,11 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, > > >> return JSMN_ERROR_PART; > > >> } > > >> > > >> +#ifdef JSMN_STRICT > > >> + return expecting_item ? JSMN_ERROR_INVAL : JSMN_SUCCESS; > > >> +#else > > >> return JSMN_SUCCESS; > > >> +#endif > > >> } > > >> > > >> /* > > >> -- > > >> 2.28.0 > > >> > > > > > -- - Arnaldo ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 2/3] perf tools: Make the JSON parser more conformant when in strict mode @ 2021-10-08 18:56 ` Arnaldo Carvalho de Melo 0 siblings, 0 replies; 36+ messages in thread From: Arnaldo Carvalho de Melo @ 2021-10-08 18:56 UTC (permalink / raw) To: Jiri Olsa Cc: James Clark, john.garry, ak, linux-perf-users, Nick.Forrington, Andrew.Kilroy, Will Deacon, Mathieu Poirier, Leo Yan, Mark Rutland, Alexander Shishkin, Namhyung Kim, linux-arm-kernel, linux-kernel Em Fri, Oct 08, 2021 at 03:12:03PM +0200, Jiri Olsa escreveu: > On Fri, Oct 08, 2021 at 11:08:25AM +0100, James Clark wrote: > > > > > > On 07/10/2021 18:52, Jiri Olsa wrote: > > > On Thu, Oct 07, 2021 at 12:05:41PM +0100, James Clark wrote: > > >> Return an error when a trailing comma is found or a new item is > > >> encountered before a comma or an opening brace. This ensures that the > > >> perf json files conform more closely to the spec at https://www.json.org > > >> > > >> Signed-off-by: James Clark <james.clark@arm.com> > > >> --- > > >> tools/perf/pmu-events/jsmn.c | 42 ++++++++++++++++++++++++++++++++++-- > > >> 1 file changed, 40 insertions(+), 2 deletions(-) > > >> > > >> diff --git a/tools/perf/pmu-events/jsmn.c b/tools/perf/pmu-events/jsmn.c > > >> index 11d1fa18bfa5..8124d2d3ff0c 100644 > > >> --- a/tools/perf/pmu-events/jsmn.c > > >> +++ b/tools/perf/pmu-events/jsmn.c > > >> @@ -176,6 +176,14 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, > > >> jsmnerr_t r; > > >> int i; > > >> jsmntok_t *token; > > >> +#ifdef JSMN_STRICT > > > > > > I might have missed some discussion on this, but do we need the > > > JSMN_STRICT define, if you enable it in the next patch? > > > why can't we be more strict by default.. do you plan to disable > > > it in future? > > > > I didn't plan on disabling it, I was just trying to keep to the existing style of the > > jsmn project. > > > > I could have added the trailing comma detection by default and not inside any > > #ifdef JSMN_STRICT blocks, but I would like to enable JSMN_STRICT anyway, because it > > enables some additional built in checking that was already there. So I thought it > > made sense to put my new strict stuff inside the existing strict option. > > > > One option would be to remove all (including the existing) #ifdef JSMN_STRICT blocks > > and have everything strict by default. But it would be a further deviation from jsmn. > > ok, I think it makes sense to have JSMN_STRICT then.. > thanks for explanation > > Acked-by: Jiri Olsa <jolsa@redhat.com> So, is this for the whole patchset? b4 picked it just for this message. - Arnaldo > jirka > > > > > Thanks > > James > > > > > > > > thanks, > > > jirka > > > > > >> + /* > > >> + * Keeps track of whether a new object/list/primitive is expected. New items are only > > >> + * allowed after an opening brace, comma or colon. A closing brace after a comma is not > > >> + * valid JSON. > > >> + */ > > >> + int expecting_item = 1; > > >> +#endif > > >> > > >> for (; parser->pos < len; parser->pos++) { > > >> char c; > > >> @@ -185,6 +193,10 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, > > >> switch (c) { > > >> case '{': > > >> case '[': > > >> +#ifdef JSMN_STRICT > > >> + if (!expecting_item) > > >> + return JSMN_ERROR_INVAL; > > >> +#endif > > >> token = jsmn_alloc_token(parser, tokens, num_tokens); > > >> if (token == NULL) > > >> return JSMN_ERROR_NOMEM; > > >> @@ -196,6 +208,10 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, > > >> break; > > >> case '}': > > >> case ']': > > >> +#ifdef JSMN_STRICT > > >> + if (expecting_item) > > >> + return JSMN_ERROR_INVAL; > > >> +#endif > > >> type = (c == '}' ? JSMN_OBJECT : JSMN_ARRAY); > > >> for (i = parser->toknext - 1; i >= 0; i--) { > > >> token = &tokens[i]; > > >> @@ -219,6 +235,11 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, > > >> } > > >> break; > > >> case '\"': > > >> +#ifdef JSMN_STRICT > > >> + if (!expecting_item) > > >> + return JSMN_ERROR_INVAL; > > >> + expecting_item = 0; > > >> +#endif > > >> r = jsmn_parse_string(parser, js, len, tokens, > > >> num_tokens); > > >> if (r < 0) > > >> @@ -229,11 +250,15 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, > > >> case '\t': > > >> case '\r': > > >> case '\n': > > >> - case ':': > > >> - case ',': > > >> case ' ': > > >> break; > > >> #ifdef JSMN_STRICT > > >> + case ':': > > >> + case ',': > > >> + if (expecting_item) > > >> + return JSMN_ERROR_INVAL; > > >> + expecting_item = 1; > > >> + break; > > >> /* > > >> * In strict mode primitives are: > > >> * numbers and booleans. > > >> @@ -253,6 +278,9 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, > > >> case 'f': > > >> case 'n': > > >> #else > > >> + case ':': > > >> + case ',': > > >> + break; > > >> /* > > >> * In non-strict mode every unquoted value > > >> * is a primitive. > > >> @@ -260,6 +288,12 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, > > >> /*FALL THROUGH */ > > >> default: > > >> #endif > > >> + > > >> +#ifdef JSMN_STRICT > > >> + if (!expecting_item) > > >> + return JSMN_ERROR_INVAL; > > >> + expecting_item = 0; > > >> +#endif > > >> r = jsmn_parse_primitive(parser, js, len, tokens, > > >> num_tokens); > > >> if (r < 0) > > >> @@ -282,7 +316,11 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, > > >> return JSMN_ERROR_PART; > > >> } > > >> > > >> +#ifdef JSMN_STRICT > > >> + return expecting_item ? JSMN_ERROR_INVAL : JSMN_SUCCESS; > > >> +#else > > >> return JSMN_SUCCESS; > > >> +#endif > > >> } > > >> > > >> /* > > >> -- > > >> 2.28.0 > > >> > > > > > -- - Arnaldo _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 2/3] perf tools: Make the JSON parser more conformant when in strict mode 2021-10-08 18:56 ` Arnaldo Carvalho de Melo @ 2021-10-08 19:01 ` Arnaldo Carvalho de Melo -1 siblings, 0 replies; 36+ messages in thread From: Arnaldo Carvalho de Melo @ 2021-10-08 19:01 UTC (permalink / raw) To: Jiri Olsa Cc: James Clark, john.garry, ak, linux-perf-users, Nick.Forrington, Andrew.Kilroy, Will Deacon, Mathieu Poirier, Leo Yan, Mark Rutland, Alexander Shishkin, Namhyung Kim, linux-arm-kernel, linux-kernel Em Fri, Oct 08, 2021 at 03:56:26PM -0300, Arnaldo Carvalho de Melo escreveu: > Em Fri, Oct 08, 2021 at 03:12:03PM +0200, Jiri Olsa escreveu: > > On Fri, Oct 08, 2021 at 11:08:25AM +0100, James Clark wrote: > > > > > > > > > On 07/10/2021 18:52, Jiri Olsa wrote: > > > > On Thu, Oct 07, 2021 at 12:05:41PM +0100, James Clark wrote: > > > >> Return an error when a trailing comma is found or a new item is > > > >> encountered before a comma or an opening brace. This ensures that the > > > >> perf json files conform more closely to the spec at https://www.json.org > > > >> > > > >> Signed-off-by: James Clark <james.clark@arm.com> > > > >> --- > > > >> tools/perf/pmu-events/jsmn.c | 42 ++++++++++++++++++++++++++++++++++-- > > > >> 1 file changed, 40 insertions(+), 2 deletions(-) > > > >> > > > >> diff --git a/tools/perf/pmu-events/jsmn.c b/tools/perf/pmu-events/jsmn.c > > > >> index 11d1fa18bfa5..8124d2d3ff0c 100644 > > > >> --- a/tools/perf/pmu-events/jsmn.c > > > >> +++ b/tools/perf/pmu-events/jsmn.c > > > >> @@ -176,6 +176,14 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, > > > >> jsmnerr_t r; > > > >> int i; > > > >> jsmntok_t *token; > > > >> +#ifdef JSMN_STRICT > > > > > > > > I might have missed some discussion on this, but do we need the > > > > JSMN_STRICT define, if you enable it in the next patch? > > > > why can't we be more strict by default.. do you plan to disable > > > > it in future? > > > > > > I didn't plan on disabling it, I was just trying to keep to the existing style of the > > > jsmn project. > > > > > > I could have added the trailing comma detection by default and not inside any > > > #ifdef JSMN_STRICT blocks, but I would like to enable JSMN_STRICT anyway, because it > > > enables some additional built in checking that was already there. So I thought it > > > made sense to put my new strict stuff inside the existing strict option. > > > > > > One option would be to remove all (including the existing) #ifdef JSMN_STRICT blocks > > > and have everything strict by default. But it would be a further deviation from jsmn. > > > > ok, I think it makes sense to have JSMN_STRICT then.. > > thanks for explanation > > > > Acked-by: Jiri Olsa <jolsa@redhat.com> > > So, is this for the whole patchset? b4 picked it just for this message. Got it from IRC, thanks, - Arnaldo ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 2/3] perf tools: Make the JSON parser more conformant when in strict mode @ 2021-10-08 19:01 ` Arnaldo Carvalho de Melo 0 siblings, 0 replies; 36+ messages in thread From: Arnaldo Carvalho de Melo @ 2021-10-08 19:01 UTC (permalink / raw) To: Jiri Olsa Cc: James Clark, john.garry, ak, linux-perf-users, Nick.Forrington, Andrew.Kilroy, Will Deacon, Mathieu Poirier, Leo Yan, Mark Rutland, Alexander Shishkin, Namhyung Kim, linux-arm-kernel, linux-kernel Em Fri, Oct 08, 2021 at 03:56:26PM -0300, Arnaldo Carvalho de Melo escreveu: > Em Fri, Oct 08, 2021 at 03:12:03PM +0200, Jiri Olsa escreveu: > > On Fri, Oct 08, 2021 at 11:08:25AM +0100, James Clark wrote: > > > > > > > > > On 07/10/2021 18:52, Jiri Olsa wrote: > > > > On Thu, Oct 07, 2021 at 12:05:41PM +0100, James Clark wrote: > > > >> Return an error when a trailing comma is found or a new item is > > > >> encountered before a comma or an opening brace. This ensures that the > > > >> perf json files conform more closely to the spec at https://www.json.org > > > >> > > > >> Signed-off-by: James Clark <james.clark@arm.com> > > > >> --- > > > >> tools/perf/pmu-events/jsmn.c | 42 ++++++++++++++++++++++++++++++++++-- > > > >> 1 file changed, 40 insertions(+), 2 deletions(-) > > > >> > > > >> diff --git a/tools/perf/pmu-events/jsmn.c b/tools/perf/pmu-events/jsmn.c > > > >> index 11d1fa18bfa5..8124d2d3ff0c 100644 > > > >> --- a/tools/perf/pmu-events/jsmn.c > > > >> +++ b/tools/perf/pmu-events/jsmn.c > > > >> @@ -176,6 +176,14 @@ jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, size_t len, > > > >> jsmnerr_t r; > > > >> int i; > > > >> jsmntok_t *token; > > > >> +#ifdef JSMN_STRICT > > > > > > > > I might have missed some discussion on this, but do we need the > > > > JSMN_STRICT define, if you enable it in the next patch? > > > > why can't we be more strict by default.. do you plan to disable > > > > it in future? > > > > > > I didn't plan on disabling it, I was just trying to keep to the existing style of the > > > jsmn project. > > > > > > I could have added the trailing comma detection by default and not inside any > > > #ifdef JSMN_STRICT blocks, but I would like to enable JSMN_STRICT anyway, because it > > > enables some additional built in checking that was already there. So I thought it > > > made sense to put my new strict stuff inside the existing strict option. > > > > > > One option would be to remove all (including the existing) #ifdef JSMN_STRICT blocks > > > and have everything strict by default. But it would be a further deviation from jsmn. > > > > ok, I think it makes sense to have JSMN_STRICT then.. > > thanks for explanation > > > > Acked-by: Jiri Olsa <jolsa@redhat.com> > > So, is this for the whole patchset? b4 picked it just for this message. Got it from IRC, thanks, - Arnaldo _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 3/3] perf tools: Enable strict JSON parsing 2021-10-07 11:05 ` James Clark @ 2021-10-07 11:05 ` James Clark -1 siblings, 0 replies; 36+ messages in thread From: James Clark @ 2021-10-07 11:05 UTC (permalink / raw) To: acme, john.garry, ak, linux-perf-users Cc: Nick.Forrington, Andrew.Kilroy, James Clark, Will Deacon, Mathieu Poirier, Leo Yan, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim, linux-arm-kernel, linux-kernel This is to ensure that the PMU event files can always be parsed by other tools. Testing ======= * There are no errors when parsing files for all architectures: # pmu-events/jevents nds32 pmu-events/arch/ test # pmu-events/jevents s390 pmu-events/arch/ test # pmu-events/jevents powerpc pmu-events/arch/ test # pmu-events/jevents arm64 pmu-events/arch/ test # pmu-events/jevents test pmu-events/arch/ test # pmu-events/jevents x86 pmu-events/arch/ test * Trailing and leading commas now cause a parse error * Double commas now cause a parse error * Compilation and parsing works with strict mode disabled and enabled * A diff of the output files shows no changes Signed-off-by: James Clark <james.clark@arm.com> --- tools/perf/pmu-events/jsmn.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/perf/pmu-events/jsmn.c b/tools/perf/pmu-events/jsmn.c index 8124d2d3ff0c..831dc44c4558 100644 --- a/tools/perf/pmu-events/jsmn.c +++ b/tools/perf/pmu-events/jsmn.c @@ -24,6 +24,7 @@ #include <stdlib.h> #include "jsmn.h" +#define JSMN_STRICT /* * Allocates a fresh unused token from the token pool. -- 2.28.0 ^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH 3/3] perf tools: Enable strict JSON parsing @ 2021-10-07 11:05 ` James Clark 0 siblings, 0 replies; 36+ messages in thread From: James Clark @ 2021-10-07 11:05 UTC (permalink / raw) To: acme, john.garry, ak, linux-perf-users Cc: Nick.Forrington, Andrew.Kilroy, James Clark, Will Deacon, Mathieu Poirier, Leo Yan, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim, linux-arm-kernel, linux-kernel This is to ensure that the PMU event files can always be parsed by other tools. Testing ======= * There are no errors when parsing files for all architectures: # pmu-events/jevents nds32 pmu-events/arch/ test # pmu-events/jevents s390 pmu-events/arch/ test # pmu-events/jevents powerpc pmu-events/arch/ test # pmu-events/jevents arm64 pmu-events/arch/ test # pmu-events/jevents test pmu-events/arch/ test # pmu-events/jevents x86 pmu-events/arch/ test * Trailing and leading commas now cause a parse error * Double commas now cause a parse error * Compilation and parsing works with strict mode disabled and enabled * A diff of the output files shows no changes Signed-off-by: James Clark <james.clark@arm.com> --- tools/perf/pmu-events/jsmn.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/perf/pmu-events/jsmn.c b/tools/perf/pmu-events/jsmn.c index 8124d2d3ff0c..831dc44c4558 100644 --- a/tools/perf/pmu-events/jsmn.c +++ b/tools/perf/pmu-events/jsmn.c @@ -24,6 +24,7 @@ #include <stdlib.h> #include "jsmn.h" +#define JSMN_STRICT /* * Allocates a fresh unused token from the token pool. -- 2.28.0 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [PATCH 0/3] perf tools: Enable strict JSON parsing 2021-10-07 11:05 ` James Clark @ 2021-10-07 23:51 ` Andi Kleen -1 siblings, 0 replies; 36+ messages in thread From: Andi Kleen @ 2021-10-07 23:51 UTC (permalink / raw) To: James Clark, acme, john.garry, linux-perf-users Cc: Nick.Forrington, Andrew.Kilroy, Will Deacon, Mathieu Poirier, Leo Yan, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim, linux-arm-kernel, linux-kernel On 10/7/2021 4:05 AM, James Clark wrote: > After a discussion on "[PATCH 1/4] perf vendor events: Syntax corrections in Neoverse N1 json", > John Garry suggested that we can just modify the parser to make it more strict. Hopefully this will > remove the need to apply any future JSON comma fixup commits. > > Applies on top of "[PATCH v2 1/3] perf vendor events: Syntax corrections in Neoverse N1 json" on > perf/core. Looks good to me. The Intel files are already generated by automated tools using the standard python JSON writer, I guess if it's out of sync someone must have edited it by hand. So it should be fine to fix it. Reviewed-by: Andi Kleen <ak@linux.intel.com> -Andi ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 0/3] perf tools: Enable strict JSON parsing @ 2021-10-07 23:51 ` Andi Kleen 0 siblings, 0 replies; 36+ messages in thread From: Andi Kleen @ 2021-10-07 23:51 UTC (permalink / raw) To: James Clark, acme, john.garry, linux-perf-users Cc: Nick.Forrington, Andrew.Kilroy, Will Deacon, Mathieu Poirier, Leo Yan, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim, linux-arm-kernel, linux-kernel On 10/7/2021 4:05 AM, James Clark wrote: > After a discussion on "[PATCH 1/4] perf vendor events: Syntax corrections in Neoverse N1 json", > John Garry suggested that we can just modify the parser to make it more strict. Hopefully this will > remove the need to apply any future JSON comma fixup commits. > > Applies on top of "[PATCH v2 1/3] perf vendor events: Syntax corrections in Neoverse N1 json" on > perf/core. Looks good to me. The Intel files are already generated by automated tools using the standard python JSON writer, I guess if it's out of sync someone must have edited it by hand. So it should be fine to fix it. Reviewed-by: Andi Kleen <ak@linux.intel.com> -Andi _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 0/3] perf tools: Enable strict JSON parsing 2021-10-07 11:05 ` James Clark @ 2021-10-08 7:43 ` kajoljain -1 siblings, 0 replies; 36+ messages in thread From: kajoljain @ 2021-10-08 7:43 UTC (permalink / raw) To: James Clark, acme, john.garry, ak, linux-perf-users Cc: Nick.Forrington, Andrew.Kilroy, Will Deacon, Mathieu Poirier, Leo Yan, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim, linux-arm-kernel, linux-kernel On 10/7/21 4:35 PM, James Clark wrote: > After a discussion on "[PATCH 1/4] perf vendor events: Syntax corrections in Neoverse N1 json", > John Garry suggested that we can just modify the parser to make it more strict. Hopefully this will > remove the need to apply any future JSON comma fixup commits. > > Applies on top of "[PATCH v2 1/3] perf vendor events: Syntax corrections in Neoverse N1 json" on > perf/core. > > Also available at: > git clone --branch james-json-parse-fix git@git.gitlab.arm.com:linux-arm/linux-jc.git Hi James, Do we have any dependency patches on top of this patch series. I am reviewing and testing it, but in both powerpc and x86 system I am getting build issue. Not sure if I am missing something. I am trying your changes on top of upstream perf. pmu-events/arch/test/test_soc/sys/uncore.json: json error Invalid character inside JSON string make[3]: *** [pmu-events/Build:18: pmu-events/pmu-events.c] Error 1 make[3]: *** Deleting file 'pmu-events/pmu-events.c' make[2]: *** [Makefile.perf:667: pmu-events/pmu-events-in.o] Error 2 make[2]: *** Waiting for unfinished jobs.... make[1]: *** [Makefile.perf:238: sub-make] Error 2 make: *** [Makefile:70: all] Error 2 Also, Is it possible to add line number along with file name while showing this error `json error Invalid character inside JSON string`. It might make it easy to fix. Thanks, Kajol Jain > > James Clark (3): > perf vendor-events: Fix all remaining invalid JSON files > perf tools: Make the JSON parser more conformant when in strict mode > perf tools: Enable strict JSON parsing > > .../arch/arm64/ampere/emag/bus.json | 2 +- > .../arch/arm64/ampere/emag/cache.json | 20 ++++----- > .../arch/arm64/ampere/emag/clock.json | 2 +- > .../arch/arm64/ampere/emag/exception.json | 4 +- > .../arch/arm64/ampere/emag/instruction.json | 10 ++--- > .../arch/arm64/ampere/emag/memory.json | 4 +- > .../arch/arm64/hisilicon/hip08/metrics.json | 2 +- > .../pmu-events/arch/nds32/n13/atcpmu.json | 2 +- > .../pmu-events/arch/s390/cf_z10/basic.json | 2 +- > .../pmu-events/arch/s390/cf_z10/crypto.json | 2 +- > .../pmu-events/arch/s390/cf_z10/extended.json | 2 +- > .../pmu-events/arch/s390/cf_z13/basic.json | 2 +- > .../pmu-events/arch/s390/cf_z13/crypto.json | 2 +- > .../pmu-events/arch/s390/cf_z13/extended.json | 2 +- > .../pmu-events/arch/s390/cf_z14/basic.json | 2 +- > .../pmu-events/arch/s390/cf_z14/crypto.json | 2 +- > .../pmu-events/arch/s390/cf_z14/extended.json | 2 +- > .../pmu-events/arch/s390/cf_z15/basic.json | 2 +- > .../pmu-events/arch/s390/cf_z15/crypto.json | 2 +- > .../pmu-events/arch/s390/cf_z15/crypto6.json | 2 +- > .../pmu-events/arch/s390/cf_z15/extended.json | 2 +- > .../pmu-events/arch/s390/cf_z196/basic.json | 2 +- > .../pmu-events/arch/s390/cf_z196/crypto.json | 2 +- > .../arch/s390/cf_z196/extended.json | 2 +- > .../pmu-events/arch/s390/cf_zec12/basic.json | 2 +- > .../pmu-events/arch/s390/cf_zec12/crypto.json | 2 +- > .../arch/s390/cf_zec12/extended.json | 2 +- > .../arch/test/test_soc/cpu/uncore.json | 2 +- > .../arch/x86/icelakex/icx-metrics.json | 2 +- > tools/perf/pmu-events/jsmn.c | 43 ++++++++++++++++++- > 30 files changed, 85 insertions(+), 46 deletions(-) > ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 0/3] perf tools: Enable strict JSON parsing @ 2021-10-08 7:43 ` kajoljain 0 siblings, 0 replies; 36+ messages in thread From: kajoljain @ 2021-10-08 7:43 UTC (permalink / raw) To: James Clark, acme, john.garry, ak, linux-perf-users Cc: Nick.Forrington, Andrew.Kilroy, Will Deacon, Mathieu Poirier, Leo Yan, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim, linux-arm-kernel, linux-kernel On 10/7/21 4:35 PM, James Clark wrote: > After a discussion on "[PATCH 1/4] perf vendor events: Syntax corrections in Neoverse N1 json", > John Garry suggested that we can just modify the parser to make it more strict. Hopefully this will > remove the need to apply any future JSON comma fixup commits. > > Applies on top of "[PATCH v2 1/3] perf vendor events: Syntax corrections in Neoverse N1 json" on > perf/core. > > Also available at: > git clone --branch james-json-parse-fix git@git.gitlab.arm.com:linux-arm/linux-jc.git Hi James, Do we have any dependency patches on top of this patch series. I am reviewing and testing it, but in both powerpc and x86 system I am getting build issue. Not sure if I am missing something. I am trying your changes on top of upstream perf. pmu-events/arch/test/test_soc/sys/uncore.json: json error Invalid character inside JSON string make[3]: *** [pmu-events/Build:18: pmu-events/pmu-events.c] Error 1 make[3]: *** Deleting file 'pmu-events/pmu-events.c' make[2]: *** [Makefile.perf:667: pmu-events/pmu-events-in.o] Error 2 make[2]: *** Waiting for unfinished jobs.... make[1]: *** [Makefile.perf:238: sub-make] Error 2 make: *** [Makefile:70: all] Error 2 Also, Is it possible to add line number along with file name while showing this error `json error Invalid character inside JSON string`. It might make it easy to fix. Thanks, Kajol Jain > > James Clark (3): > perf vendor-events: Fix all remaining invalid JSON files > perf tools: Make the JSON parser more conformant when in strict mode > perf tools: Enable strict JSON parsing > > .../arch/arm64/ampere/emag/bus.json | 2 +- > .../arch/arm64/ampere/emag/cache.json | 20 ++++----- > .../arch/arm64/ampere/emag/clock.json | 2 +- > .../arch/arm64/ampere/emag/exception.json | 4 +- > .../arch/arm64/ampere/emag/instruction.json | 10 ++--- > .../arch/arm64/ampere/emag/memory.json | 4 +- > .../arch/arm64/hisilicon/hip08/metrics.json | 2 +- > .../pmu-events/arch/nds32/n13/atcpmu.json | 2 +- > .../pmu-events/arch/s390/cf_z10/basic.json | 2 +- > .../pmu-events/arch/s390/cf_z10/crypto.json | 2 +- > .../pmu-events/arch/s390/cf_z10/extended.json | 2 +- > .../pmu-events/arch/s390/cf_z13/basic.json | 2 +- > .../pmu-events/arch/s390/cf_z13/crypto.json | 2 +- > .../pmu-events/arch/s390/cf_z13/extended.json | 2 +- > .../pmu-events/arch/s390/cf_z14/basic.json | 2 +- > .../pmu-events/arch/s390/cf_z14/crypto.json | 2 +- > .../pmu-events/arch/s390/cf_z14/extended.json | 2 +- > .../pmu-events/arch/s390/cf_z15/basic.json | 2 +- > .../pmu-events/arch/s390/cf_z15/crypto.json | 2 +- > .../pmu-events/arch/s390/cf_z15/crypto6.json | 2 +- > .../pmu-events/arch/s390/cf_z15/extended.json | 2 +- > .../pmu-events/arch/s390/cf_z196/basic.json | 2 +- > .../pmu-events/arch/s390/cf_z196/crypto.json | 2 +- > .../arch/s390/cf_z196/extended.json | 2 +- > .../pmu-events/arch/s390/cf_zec12/basic.json | 2 +- > .../pmu-events/arch/s390/cf_zec12/crypto.json | 2 +- > .../arch/s390/cf_zec12/extended.json | 2 +- > .../arch/test/test_soc/cpu/uncore.json | 2 +- > .../arch/x86/icelakex/icx-metrics.json | 2 +- > tools/perf/pmu-events/jsmn.c | 43 ++++++++++++++++++- > 30 files changed, 85 insertions(+), 46 deletions(-) > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 0/3] perf tools: Enable strict JSON parsing 2021-10-08 7:43 ` kajoljain @ 2021-10-08 10:02 ` James Clark -1 siblings, 0 replies; 36+ messages in thread From: James Clark @ 2021-10-08 10:02 UTC (permalink / raw) To: kajoljain, acme, john.garry, ak, linux-perf-users Cc: Nick.Forrington, Andrew.Kilroy, Will Deacon, Mathieu Poirier, Leo Yan, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim, linux-arm-kernel, linux-kernel On 08/10/2021 08:43, kajoljain wrote: > > > On 10/7/21 4:35 PM, James Clark wrote: >> After a discussion on "[PATCH 1/4] perf vendor events: Syntax corrections in Neoverse N1 json", >> John Garry suggested that we can just modify the parser to make it more strict. Hopefully this will >> remove the need to apply any future JSON comma fixup commits. >> >> Applies on top of "[PATCH v2 1/3] perf vendor events: Syntax corrections in Neoverse N1 json" on >> perf/core. >> >> Also available at: >> git clone --branch james-json-parse-fix git@git.gitlab.arm.com:linux-arm/linux-jc.git > > Hi James, > Do we have any dependency patches on top of this patch series. I am > reviewing and testing it, but in both powerpc and x86 system I am > getting build issue. Not sure if I am missing something> > I am trying your changes on top of upstream perf. > > pmu-events/arch/test/test_soc/sys/uncore.json: json error Invalid > character inside JSON string Hi Kajol, A trailing comma was fixed in this file 3 weeks ago at b8b350a. Can you confirm if you have updated to get this commit on perf core? Alternately you could pull from my branch above which is up to date enough to include it. The file is in pmu-events/arch/test/ so I would expect it to fail on all platforms. > make[3]: *** [pmu-events/Build:18: pmu-events/pmu-events.c] Error 1 > make[3]: *** Deleting file 'pmu-events/pmu-events.c' > make[2]: *** [Makefile.perf:667: pmu-events/pmu-events-in.o] Error 2 > make[2]: *** Waiting for unfinished jobs.... > make[1]: *** [Makefile.perf:238: sub-make] Error 2 > make: *** [Makefile:70: all] Error 2 > > Also, Is it possible to add line number along with file name while > showing this error `json error Invalid character inside JSON string`. > It might make it easy to fix. I can add a character number with the following fix if you think that would be good enough? A line number might be a bigger change and involve keeping track of newline characters. diff --git a/tools/perf/pmu-events/json.c b/tools/perf/pmu-events/json.c index 0544398d6e2d..41a14e1543bf 100644 --- a/tools/perf/pmu-events/json.c +++ b/tools/perf/pmu-events/json.c @@ -99,7 +99,7 @@ jsmntok_t *parse_json(const char *fn, char **map, size_t *size, int *len) res = jsmn_parse(&parser, *map, *size, tokens, sz / sizeof(jsmntok_t)); if (res != JSMN_SUCCESS) { - pr_err("%s: json error %s\n", fn, jsmn_strerror(res)); + pr_err("%s: json error at character %u '%s'\n", fn, parser.pos, jsmn_strerror(res)); goto error_free; } if (len) It prints this for the same error you have above: pmu-events/arch/test/test_soc/sys/uncore.json: json error at character 213 'Invalid character inside JSON string' Although funnily enough after re-introducing that extra comma it doesn't fail the build for me, it just prints the error message. But I may have noticed some dependency tracking issues around the json files. James ^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [PATCH 0/3] perf tools: Enable strict JSON parsing @ 2021-10-08 10:02 ` James Clark 0 siblings, 0 replies; 36+ messages in thread From: James Clark @ 2021-10-08 10:02 UTC (permalink / raw) To: kajoljain, acme, john.garry, ak, linux-perf-users Cc: Nick.Forrington, Andrew.Kilroy, Will Deacon, Mathieu Poirier, Leo Yan, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim, linux-arm-kernel, linux-kernel On 08/10/2021 08:43, kajoljain wrote: > > > On 10/7/21 4:35 PM, James Clark wrote: >> After a discussion on "[PATCH 1/4] perf vendor events: Syntax corrections in Neoverse N1 json", >> John Garry suggested that we can just modify the parser to make it more strict. Hopefully this will >> remove the need to apply any future JSON comma fixup commits. >> >> Applies on top of "[PATCH v2 1/3] perf vendor events: Syntax corrections in Neoverse N1 json" on >> perf/core. >> >> Also available at: >> git clone --branch james-json-parse-fix git@git.gitlab.arm.com:linux-arm/linux-jc.git > > Hi James, > Do we have any dependency patches on top of this patch series. I am > reviewing and testing it, but in both powerpc and x86 system I am > getting build issue. Not sure if I am missing something> > I am trying your changes on top of upstream perf. > > pmu-events/arch/test/test_soc/sys/uncore.json: json error Invalid > character inside JSON string Hi Kajol, A trailing comma was fixed in this file 3 weeks ago at b8b350a. Can you confirm if you have updated to get this commit on perf core? Alternately you could pull from my branch above which is up to date enough to include it. The file is in pmu-events/arch/test/ so I would expect it to fail on all platforms. > make[3]: *** [pmu-events/Build:18: pmu-events/pmu-events.c] Error 1 > make[3]: *** Deleting file 'pmu-events/pmu-events.c' > make[2]: *** [Makefile.perf:667: pmu-events/pmu-events-in.o] Error 2 > make[2]: *** Waiting for unfinished jobs.... > make[1]: *** [Makefile.perf:238: sub-make] Error 2 > make: *** [Makefile:70: all] Error 2 > > Also, Is it possible to add line number along with file name while > showing this error `json error Invalid character inside JSON string`. > It might make it easy to fix. I can add a character number with the following fix if you think that would be good enough? A line number might be a bigger change and involve keeping track of newline characters. diff --git a/tools/perf/pmu-events/json.c b/tools/perf/pmu-events/json.c index 0544398d6e2d..41a14e1543bf 100644 --- a/tools/perf/pmu-events/json.c +++ b/tools/perf/pmu-events/json.c @@ -99,7 +99,7 @@ jsmntok_t *parse_json(const char *fn, char **map, size_t *size, int *len) res = jsmn_parse(&parser, *map, *size, tokens, sz / sizeof(jsmntok_t)); if (res != JSMN_SUCCESS) { - pr_err("%s: json error %s\n", fn, jsmn_strerror(res)); + pr_err("%s: json error at character %u '%s'\n", fn, parser.pos, jsmn_strerror(res)); goto error_free; } if (len) It prints this for the same error you have above: pmu-events/arch/test/test_soc/sys/uncore.json: json error at character 213 'Invalid character inside JSON string' Although funnily enough after re-introducing that extra comma it doesn't fail the build for me, it just prints the error message. But I may have noticed some dependency tracking issues around the json files. James _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [PATCH 0/3] perf tools: Enable strict JSON parsing 2021-10-08 10:02 ` James Clark @ 2021-10-08 11:26 ` kajoljain -1 siblings, 0 replies; 36+ messages in thread From: kajoljain @ 2021-10-08 11:26 UTC (permalink / raw) To: James Clark, acme, john.garry, ak, linux-perf-users Cc: Nick.Forrington, Andrew.Kilroy, Will Deacon, Mathieu Poirier, Leo Yan, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim, linux-arm-kernel, linux-kernel On 10/8/21 3:32 PM, James Clark wrote: > > > On 08/10/2021 08:43, kajoljain wrote: >> >> >> On 10/7/21 4:35 PM, James Clark wrote: >>> After a discussion on "[PATCH 1/4] perf vendor events: Syntax corrections in Neoverse N1 json", >>> John Garry suggested that we can just modify the parser to make it more strict. Hopefully this will >>> remove the need to apply any future JSON comma fixup commits. >>> >>> Applies on top of "[PATCH v2 1/3] perf vendor events: Syntax corrections in Neoverse N1 json" on >>> perf/core. >>> >>> Also available at: >>> git clone --branch james-json-parse-fix git@git.gitlab.arm.com:linux-arm/linux-jc.git >> >> Hi James, >> Do we have any dependency patches on top of this patch series. I am >> reviewing and testing it, but in both powerpc and x86 system I am >> getting build issue. Not sure if I am missing something> >> I am trying your changes on top of upstream perf. >> >> pmu-events/arch/test/test_soc/sys/uncore.json: json error Invalid >> character inside JSON string > > Hi Kajol, > > A trailing comma was fixed in this file 3 weeks ago at b8b350a. Can you > confirm if you have updated to get this commit on perf core? > > Alternately you could pull from my branch above which is up to date enough > to include it. Hi James, Thanks for pointing it. Not getting build issue now. > > The file is in pmu-events/arch/test/ so I would expect it to fail on all platforms. > >> make[3]: *** [pmu-events/Build:18: pmu-events/pmu-events.c] Error 1 >> make[3]: *** Deleting file 'pmu-events/pmu-events.c' >> make[2]: *** [Makefile.perf:667: pmu-events/pmu-events-in.o] Error 2 >> make[2]: *** Waiting for unfinished jobs.... >> make[1]: *** [Makefile.perf:238: sub-make] Error 2 >> make: *** [Makefile:70: all] Error 2 >> >> Also, Is it possible to add line number along with file name while >> showing this error `json error Invalid character inside JSON string`. >> It might make it easy to fix. > > I can add a character number with the following fix if you think that would > be good enough? A line number might be a bigger change and involve keeping > track of newline characters. Sure. I think then we can skip this change. Not sure if character number will be helpful. Patch-set looks good to me. Reviewed-by Kajol Jain<kjain@linux.ibm.com> Thanks, Kajol Jain > > diff --git a/tools/perf/pmu-events/json.c b/tools/perf/pmu-events/json.c > index 0544398d6e2d..41a14e1543bf 100644 > --- a/tools/perf/pmu-events/json.c > +++ b/tools/perf/pmu-events/json.c > @@ -99,7 +99,7 @@ jsmntok_t *parse_json(const char *fn, char **map, size_t *size, int *len) > res = jsmn_parse(&parser, *map, *size, tokens, > sz / sizeof(jsmntok_t)); > if (res != JSMN_SUCCESS) { > - pr_err("%s: json error %s\n", fn, jsmn_strerror(res)); > + pr_err("%s: json error at character %u '%s'\n", fn, parser.pos, jsmn_strerror(res)); > goto error_free; > } > if (len) > > > It prints this for the same error you have above> > pmu-events/arch/test/test_soc/sys/uncore.json: json error at character 213 'Invalid character inside JSON string' > > Although funnily enough after re-introducing that extra comma it doesn't fail the build for me, > it just prints the error message. But I may have noticed some dependency tracking issues around > the json files. > > James > ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 0/3] perf tools: Enable strict JSON parsing @ 2021-10-08 11:26 ` kajoljain 0 siblings, 0 replies; 36+ messages in thread From: kajoljain @ 2021-10-08 11:26 UTC (permalink / raw) To: James Clark, acme, john.garry, ak, linux-perf-users Cc: Nick.Forrington, Andrew.Kilroy, Will Deacon, Mathieu Poirier, Leo Yan, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim, linux-arm-kernel, linux-kernel On 10/8/21 3:32 PM, James Clark wrote: > > > On 08/10/2021 08:43, kajoljain wrote: >> >> >> On 10/7/21 4:35 PM, James Clark wrote: >>> After a discussion on "[PATCH 1/4] perf vendor events: Syntax corrections in Neoverse N1 json", >>> John Garry suggested that we can just modify the parser to make it more strict. Hopefully this will >>> remove the need to apply any future JSON comma fixup commits. >>> >>> Applies on top of "[PATCH v2 1/3] perf vendor events: Syntax corrections in Neoverse N1 json" on >>> perf/core. >>> >>> Also available at: >>> git clone --branch james-json-parse-fix git@git.gitlab.arm.com:linux-arm/linux-jc.git >> >> Hi James, >> Do we have any dependency patches on top of this patch series. I am >> reviewing and testing it, but in both powerpc and x86 system I am >> getting build issue. Not sure if I am missing something> >> I am trying your changes on top of upstream perf. >> >> pmu-events/arch/test/test_soc/sys/uncore.json: json error Invalid >> character inside JSON string > > Hi Kajol, > > A trailing comma was fixed in this file 3 weeks ago at b8b350a. Can you > confirm if you have updated to get this commit on perf core? > > Alternately you could pull from my branch above which is up to date enough > to include it. Hi James, Thanks for pointing it. Not getting build issue now. > > The file is in pmu-events/arch/test/ so I would expect it to fail on all platforms. > >> make[3]: *** [pmu-events/Build:18: pmu-events/pmu-events.c] Error 1 >> make[3]: *** Deleting file 'pmu-events/pmu-events.c' >> make[2]: *** [Makefile.perf:667: pmu-events/pmu-events-in.o] Error 2 >> make[2]: *** Waiting for unfinished jobs.... >> make[1]: *** [Makefile.perf:238: sub-make] Error 2 >> make: *** [Makefile:70: all] Error 2 >> >> Also, Is it possible to add line number along with file name while >> showing this error `json error Invalid character inside JSON string`. >> It might make it easy to fix. > > I can add a character number with the following fix if you think that would > be good enough? A line number might be a bigger change and involve keeping > track of newline characters. Sure. I think then we can skip this change. Not sure if character number will be helpful. Patch-set looks good to me. Reviewed-by Kajol Jain<kjain@linux.ibm.com> Thanks, Kajol Jain > > diff --git a/tools/perf/pmu-events/json.c b/tools/perf/pmu-events/json.c > index 0544398d6e2d..41a14e1543bf 100644 > --- a/tools/perf/pmu-events/json.c > +++ b/tools/perf/pmu-events/json.c > @@ -99,7 +99,7 @@ jsmntok_t *parse_json(const char *fn, char **map, size_t *size, int *len) > res = jsmn_parse(&parser, *map, *size, tokens, > sz / sizeof(jsmntok_t)); > if (res != JSMN_SUCCESS) { > - pr_err("%s: json error %s\n", fn, jsmn_strerror(res)); > + pr_err("%s: json error at character %u '%s'\n", fn, parser.pos, jsmn_strerror(res)); > goto error_free; > } > if (len) > > > It prints this for the same error you have above> > pmu-events/arch/test/test_soc/sys/uncore.json: json error at character 213 'Invalid character inside JSON string' > > Although funnily enough after re-introducing that extra comma it doesn't fail the build for me, > it just prints the error message. But I may have noticed some dependency tracking issues around > the json files. > > James > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 0/3] perf tools: Enable strict JSON parsing 2021-10-08 11:26 ` kajoljain @ 2021-10-08 19:00 ` Arnaldo Carvalho de Melo -1 siblings, 0 replies; 36+ messages in thread From: Arnaldo Carvalho de Melo @ 2021-10-08 19:00 UTC (permalink / raw) To: kajoljain Cc: James Clark, john.garry, ak, linux-perf-users, Nick.Forrington, Andrew.Kilroy, Will Deacon, Mathieu Poirier, Leo Yan, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim, linux-arm-kernel, linux-kernel Em Fri, Oct 08, 2021 at 04:56:55PM +0530, kajoljain escreveu: > > > On 10/8/21 3:32 PM, James Clark wrote: > > > > > > On 08/10/2021 08:43, kajoljain wrote: > >> > >> > >> On 10/7/21 4:35 PM, James Clark wrote: > >>> After a discussion on "[PATCH 1/4] perf vendor events: Syntax corrections in Neoverse N1 json", > >>> John Garry suggested that we can just modify the parser to make it more strict. Hopefully this will > >>> remove the need to apply any future JSON comma fixup commits. > >>> > >>> Applies on top of "[PATCH v2 1/3] perf vendor events: Syntax corrections in Neoverse N1 json" on > >>> perf/core. > >>> > >>> Also available at: > >>> git clone --branch james-json-parse-fix git@git.gitlab.arm.com:linux-arm/linux-jc.git > >> > >> Hi James, > >> Do we have any dependency patches on top of this patch series. I am > >> reviewing and testing it, but in both powerpc and x86 system I am > >> getting build issue. Not sure if I am missing something> > >> I am trying your changes on top of upstream perf. > >> > >> pmu-events/arch/test/test_soc/sys/uncore.json: json error Invalid > >> character inside JSON string > > > > Hi Kajol, > > > > A trailing comma was fixed in this file 3 weeks ago at b8b350a. Can you > > confirm if you have updated to get this commit on perf core? > > > > Alternately you could pull from my branch above which is up to date enough > > to include it. > > Hi James, > Thanks for pointing it. Not getting build issue now. > > > > The file is in pmu-events/arch/test/ so I would expect it to fail on all platforms. > > > >> make[3]: *** [pmu-events/Build:18: pmu-events/pmu-events.c] Error 1 > >> make[3]: *** Deleting file 'pmu-events/pmu-events.c' > >> make[2]: *** [Makefile.perf:667: pmu-events/pmu-events-in.o] Error 2 > >> make[2]: *** Waiting for unfinished jobs.... > >> make[1]: *** [Makefile.perf:238: sub-make] Error 2 > >> make: *** [Makefile:70: all] Error 2 > >> > >> Also, Is it possible to add line number along with file name while > >> showing this error `json error Invalid character inside JSON string`. > >> It might make it easy to fix. > > > > I can add a character number with the following fix if you think that would > > be good enough? A line number might be a bigger change and involve keeping > > track of newline characters. > > Sure. I think then we can skip this change. Not sure if character > number will be helpful. > > Patch-set looks good to me. > > Reviewed-by Kajol Jain<kjain@linux.ibm.com> Applied ok as-is to my perf/core branch, applied and added your Reviewed-by, thanks. - Arnaldo > Thanks, > Kajol Jain > > > > > diff --git a/tools/perf/pmu-events/json.c b/tools/perf/pmu-events/json.c > > index 0544398d6e2d..41a14e1543bf 100644 > > --- a/tools/perf/pmu-events/json.c > > +++ b/tools/perf/pmu-events/json.c > > @@ -99,7 +99,7 @@ jsmntok_t *parse_json(const char *fn, char **map, size_t *size, int *len) > > res = jsmn_parse(&parser, *map, *size, tokens, > > sz / sizeof(jsmntok_t)); > > if (res != JSMN_SUCCESS) { > > - pr_err("%s: json error %s\n", fn, jsmn_strerror(res)); > > + pr_err("%s: json error at character %u '%s'\n", fn, parser.pos, jsmn_strerror(res)); > > goto error_free; > > } > > if (len) > > > > > > It prints this for the same error you have above> > > pmu-events/arch/test/test_soc/sys/uncore.json: json error at character 213 'Invalid character inside JSON string' > > > > Although funnily enough after re-introducing that extra comma it doesn't fail the build for me, > > it just prints the error message. But I may have noticed some dependency tracking issues around > > the json files. > > > > James > > -- - Arnaldo ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 0/3] perf tools: Enable strict JSON parsing @ 2021-10-08 19:00 ` Arnaldo Carvalho de Melo 0 siblings, 0 replies; 36+ messages in thread From: Arnaldo Carvalho de Melo @ 2021-10-08 19:00 UTC (permalink / raw) To: kajoljain Cc: James Clark, john.garry, ak, linux-perf-users, Nick.Forrington, Andrew.Kilroy, Will Deacon, Mathieu Poirier, Leo Yan, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim, linux-arm-kernel, linux-kernel Em Fri, Oct 08, 2021 at 04:56:55PM +0530, kajoljain escreveu: > > > On 10/8/21 3:32 PM, James Clark wrote: > > > > > > On 08/10/2021 08:43, kajoljain wrote: > >> > >> > >> On 10/7/21 4:35 PM, James Clark wrote: > >>> After a discussion on "[PATCH 1/4] perf vendor events: Syntax corrections in Neoverse N1 json", > >>> John Garry suggested that we can just modify the parser to make it more strict. Hopefully this will > >>> remove the need to apply any future JSON comma fixup commits. > >>> > >>> Applies on top of "[PATCH v2 1/3] perf vendor events: Syntax corrections in Neoverse N1 json" on > >>> perf/core. > >>> > >>> Also available at: > >>> git clone --branch james-json-parse-fix git@git.gitlab.arm.com:linux-arm/linux-jc.git > >> > >> Hi James, > >> Do we have any dependency patches on top of this patch series. I am > >> reviewing and testing it, but in both powerpc and x86 system I am > >> getting build issue. Not sure if I am missing something> > >> I am trying your changes on top of upstream perf. > >> > >> pmu-events/arch/test/test_soc/sys/uncore.json: json error Invalid > >> character inside JSON string > > > > Hi Kajol, > > > > A trailing comma was fixed in this file 3 weeks ago at b8b350a. Can you > > confirm if you have updated to get this commit on perf core? > > > > Alternately you could pull from my branch above which is up to date enough > > to include it. > > Hi James, > Thanks for pointing it. Not getting build issue now. > > > > The file is in pmu-events/arch/test/ so I would expect it to fail on all platforms. > > > >> make[3]: *** [pmu-events/Build:18: pmu-events/pmu-events.c] Error 1 > >> make[3]: *** Deleting file 'pmu-events/pmu-events.c' > >> make[2]: *** [Makefile.perf:667: pmu-events/pmu-events-in.o] Error 2 > >> make[2]: *** Waiting for unfinished jobs.... > >> make[1]: *** [Makefile.perf:238: sub-make] Error 2 > >> make: *** [Makefile:70: all] Error 2 > >> > >> Also, Is it possible to add line number along with file name while > >> showing this error `json error Invalid character inside JSON string`. > >> It might make it easy to fix. > > > > I can add a character number with the following fix if you think that would > > be good enough? A line number might be a bigger change and involve keeping > > track of newline characters. > > Sure. I think then we can skip this change. Not sure if character > number will be helpful. > > Patch-set looks good to me. > > Reviewed-by Kajol Jain<kjain@linux.ibm.com> Applied ok as-is to my perf/core branch, applied and added your Reviewed-by, thanks. - Arnaldo > Thanks, > Kajol Jain > > > > > diff --git a/tools/perf/pmu-events/json.c b/tools/perf/pmu-events/json.c > > index 0544398d6e2d..41a14e1543bf 100644 > > --- a/tools/perf/pmu-events/json.c > > +++ b/tools/perf/pmu-events/json.c > > @@ -99,7 +99,7 @@ jsmntok_t *parse_json(const char *fn, char **map, size_t *size, int *len) > > res = jsmn_parse(&parser, *map, *size, tokens, > > sz / sizeof(jsmntok_t)); > > if (res != JSMN_SUCCESS) { > > - pr_err("%s: json error %s\n", fn, jsmn_strerror(res)); > > + pr_err("%s: json error at character %u '%s'\n", fn, parser.pos, jsmn_strerror(res)); > > goto error_free; > > } > > if (len) > > > > > > It prints this for the same error you have above> > > pmu-events/arch/test/test_soc/sys/uncore.json: json error at character 213 'Invalid character inside JSON string' > > > > Although funnily enough after re-introducing that extra comma it doesn't fail the build for me, > > it just prints the error message. But I may have noticed some dependency tracking issues around > > the json files. > > > > James > > -- - Arnaldo _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 0/3] perf tools: Enable strict JSON parsing 2021-10-08 19:00 ` Arnaldo Carvalho de Melo @ 2021-10-12 13:30 ` James Clark -1 siblings, 0 replies; 36+ messages in thread From: James Clark @ 2021-10-12 13:30 UTC (permalink / raw) To: Arnaldo Carvalho de Melo, kajoljain Cc: john.garry, ak, linux-perf-users, Nick.Forrington, Andrew.Kilroy, Will Deacon, Mathieu Poirier, Leo Yan, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim, linux-arm-kernel, linux-kernel On 08/10/2021 20:00, Arnaldo Carvalho de Melo wrote: > Em Fri, Oct 08, 2021 at 04:56:55PM +0530, kajoljain escreveu: >> >> >> On 10/8/21 3:32 PM, James Clark wrote: >>> >>> >>> On 08/10/2021 08:43, kajoljain wrote: >>>> >>>> >>>> On 10/7/21 4:35 PM, James Clark wrote: >>>>> After a discussion on "[PATCH 1/4] perf vendor events: Syntax corrections in Neoverse N1 json", >>>>> John Garry suggested that we can just modify the parser to make it more strict. Hopefully this will >>>>> remove the need to apply any future JSON comma fixup commits. >>>>> >>>>> Applies on top of "[PATCH v2 1/3] perf vendor events: Syntax corrections in Neoverse N1 json" on >>>>> perf/core. >>>>> >>>>> Also available at: >>>>> git clone --branch james-json-parse-fix git@git.gitlab.arm.com:linux-arm/linux-jc.git >>>> >>>> Hi James, >>>> Do we have any dependency patches on top of this patch series. I am >>>> reviewing and testing it, but in both powerpc and x86 system I am >>>> getting build issue. Not sure if I am missing something> >>>> I am trying your changes on top of upstream perf. >>>> >>>> pmu-events/arch/test/test_soc/sys/uncore.json: json error Invalid >>>> character inside JSON string >>> >>> Hi Kajol, >>> >>> A trailing comma was fixed in this file 3 weeks ago at b8b350a. Can you >>> confirm if you have updated to get this commit on perf core? >>> >>> Alternately you could pull from my branch above which is up to date enough >>> to include it. >> >> Hi James, >> Thanks for pointing it. Not getting build issue now. >>> >>> The file is in pmu-events/arch/test/ so I would expect it to fail on all platforms. >>> >>>> make[3]: *** [pmu-events/Build:18: pmu-events/pmu-events.c] Error 1 >>>> make[3]: *** Deleting file 'pmu-events/pmu-events.c' >>>> make[2]: *** [Makefile.perf:667: pmu-events/pmu-events-in.o] Error 2 >>>> make[2]: *** Waiting for unfinished jobs.... >>>> make[1]: *** [Makefile.perf:238: sub-make] Error 2 >>>> make: *** [Makefile:70: all] Error 2 >>>> >>>> Also, Is it possible to add line number along with file name while >>>> showing this error `json error Invalid character inside JSON string`. >>>> It might make it easy to fix. >>> >>> I can add a character number with the following fix if you think that would >>> be good enough? A line number might be a bigger change and involve keeping >>> track of newline characters. >> >> Sure. I think then we can skip this change. Not sure if character >> number will be helpful. >> >> Patch-set looks good to me. >> >> Reviewed-by Kajol Jain<kjain@linux.ibm.com> > > Applied ok as-is to my perf/core branch, applied and added your > Reviewed-by, thanks. > Thanks Arnaldo. This does mean that the arm64 build will fail until "[PATCH v2 1/3] perf vendor events: Syntax corrections in Neoverse N1 json" is applied. I think there is also an arm64 build issue with "[PATCH 02/21] perf pmu: Add const to pmu_events_map." which Andrew Kilroy has replied to. James ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 0/3] perf tools: Enable strict JSON parsing @ 2021-10-12 13:30 ` James Clark 0 siblings, 0 replies; 36+ messages in thread From: James Clark @ 2021-10-12 13:30 UTC (permalink / raw) To: Arnaldo Carvalho de Melo, kajoljain Cc: john.garry, ak, linux-perf-users, Nick.Forrington, Andrew.Kilroy, Will Deacon, Mathieu Poirier, Leo Yan, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim, linux-arm-kernel, linux-kernel On 08/10/2021 20:00, Arnaldo Carvalho de Melo wrote: > Em Fri, Oct 08, 2021 at 04:56:55PM +0530, kajoljain escreveu: >> >> >> On 10/8/21 3:32 PM, James Clark wrote: >>> >>> >>> On 08/10/2021 08:43, kajoljain wrote: >>>> >>>> >>>> On 10/7/21 4:35 PM, James Clark wrote: >>>>> After a discussion on "[PATCH 1/4] perf vendor events: Syntax corrections in Neoverse N1 json", >>>>> John Garry suggested that we can just modify the parser to make it more strict. Hopefully this will >>>>> remove the need to apply any future JSON comma fixup commits. >>>>> >>>>> Applies on top of "[PATCH v2 1/3] perf vendor events: Syntax corrections in Neoverse N1 json" on >>>>> perf/core. >>>>> >>>>> Also available at: >>>>> git clone --branch james-json-parse-fix git@git.gitlab.arm.com:linux-arm/linux-jc.git >>>> >>>> Hi James, >>>> Do we have any dependency patches on top of this patch series. I am >>>> reviewing and testing it, but in both powerpc and x86 system I am >>>> getting build issue. Not sure if I am missing something> >>>> I am trying your changes on top of upstream perf. >>>> >>>> pmu-events/arch/test/test_soc/sys/uncore.json: json error Invalid >>>> character inside JSON string >>> >>> Hi Kajol, >>> >>> A trailing comma was fixed in this file 3 weeks ago at b8b350a. Can you >>> confirm if you have updated to get this commit on perf core? >>> >>> Alternately you could pull from my branch above which is up to date enough >>> to include it. >> >> Hi James, >> Thanks for pointing it. Not getting build issue now. >>> >>> The file is in pmu-events/arch/test/ so I would expect it to fail on all platforms. >>> >>>> make[3]: *** [pmu-events/Build:18: pmu-events/pmu-events.c] Error 1 >>>> make[3]: *** Deleting file 'pmu-events/pmu-events.c' >>>> make[2]: *** [Makefile.perf:667: pmu-events/pmu-events-in.o] Error 2 >>>> make[2]: *** Waiting for unfinished jobs.... >>>> make[1]: *** [Makefile.perf:238: sub-make] Error 2 >>>> make: *** [Makefile:70: all] Error 2 >>>> >>>> Also, Is it possible to add line number along with file name while >>>> showing this error `json error Invalid character inside JSON string`. >>>> It might make it easy to fix. >>> >>> I can add a character number with the following fix if you think that would >>> be good enough? A line number might be a bigger change and involve keeping >>> track of newline characters. >> >> Sure. I think then we can skip this change. Not sure if character >> number will be helpful. >> >> Patch-set looks good to me. >> >> Reviewed-by Kajol Jain<kjain@linux.ibm.com> > > Applied ok as-is to my perf/core branch, applied and added your > Reviewed-by, thanks. > Thanks Arnaldo. This does mean that the arm64 build will fail until "[PATCH v2 1/3] perf vendor events: Syntax corrections in Neoverse N1 json" is applied. I think there is also an arm64 build issue with "[PATCH 02/21] perf pmu: Add const to pmu_events_map." which Andrew Kilroy has replied to. James _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 0/3] perf tools: Enable strict JSON parsing 2021-10-12 13:30 ` James Clark @ 2021-10-12 20:15 ` Arnaldo Carvalho de Melo -1 siblings, 0 replies; 36+ messages in thread From: Arnaldo Carvalho de Melo @ 2021-10-12 20:15 UTC (permalink / raw) To: James Clark, Arnaldo Carvalho de Melo, kajoljain Cc: john.garry, ak, linux-perf-users, Nick.Forrington, Andrew.Kilroy, Will Deacon, Mathieu Poirier, Leo Yan, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim, linux-arm-kernel, linux-kernel On October 12, 2021 10:30:51 AM GMT-03:00, James Clark <james.clark@arm.com> wrote: > > >On 08/10/2021 20:00, Arnaldo Carvalho de Melo wrote: >> Em Fri, Oct 08, 2021 at 04:56:55PM +0530, kajoljain escreveu: >>> >>> >>> On 10/8/21 3:32 PM, James Clark wrote: >>>> >>>> >>>> On 08/10/2021 08:43, kajoljain wrote: >>>>> >>>>> >>>>> On 10/7/21 4:35 PM, James Clark wrote: >>>>>> After a discussion on "[PATCH 1/4] perf vendor events: Syntax corrections in Neoverse N1 json", >>>>>> John Garry suggested that we can just modify the parser to make it more strict. Hopefully this will >>>>>> remove the need to apply any future JSON comma fixup commits. >>>>>> >>>>>> Applies on top of "[PATCH v2 1/3] perf vendor events: Syntax corrections in Neoverse N1 json" on >>>>>> perf/core. >>>>>> >>>>>> Also available at: >>>>>> git clone --branch james-json-parse-fix git@git.gitlab.arm.com:linux-arm/linux-jc.git >>>>> >>>>> Hi James, >>>>> Do we have any dependency patches on top of this patch series. I am >>>>> reviewing and testing it, but in both powerpc and x86 system I am >>>>> getting build issue. Not sure if I am missing something> >>>>> I am trying your changes on top of upstream perf. >>>>> >>>>> pmu-events/arch/test/test_soc/sys/uncore.json: json error Invalid >>>>> character inside JSON string >>>> >>>> Hi Kajol, >>>> >>>> A trailing comma was fixed in this file 3 weeks ago at b8b350a. Can you >>>> confirm if you have updated to get this commit on perf core? >>>> >>>> Alternately you could pull from my branch above which is up to date enough >>>> to include it. >>> >>> Hi James, >>> Thanks for pointing it. Not getting build issue now. >>>> >>>> The file is in pmu-events/arch/test/ so I would expect it to fail on all platforms. >>>> >>>>> make[3]: *** [pmu-events/Build:18: pmu-events/pmu-events.c] Error 1 >>>>> make[3]: *** Deleting file 'pmu-events/pmu-events.c' >>>>> make[2]: *** [Makefile.perf:667: pmu-events/pmu-events-in.o] Error 2 >>>>> make[2]: *** Waiting for unfinished jobs.... >>>>> make[1]: *** [Makefile.perf:238: sub-make] Error 2 >>>>> make: *** [Makefile:70: all] Error 2 >>>>> >>>>> Also, Is it possible to add line number along with file name while >>>>> showing this error `json error Invalid character inside JSON string`. >>>>> It might make it easy to fix. >>>> >>>> I can add a character number with the following fix if you think that would >>>> be good enough? A line number might be a bigger change and involve keeping >>>> track of newline characters. >>> >>> Sure. I think then we can skip this change. Not sure if character >>> number will be helpful. >>> >>> Patch-set looks good to me. >>> >>> Reviewed-by Kajol Jain<kjain@linux.ibm.com> >> >> Applied ok as-is to my perf/core branch, applied and added your >> Reviewed-by, thanks. >> > >Thanks Arnaldo. This does mean that the arm64 build will fail until >"[PATCH v2 1/3] perf vendor events: Syntax corrections in Neoverse N1 json" is >applied. I think there is also an arm64 build issue with "[PATCH 02/21] perf >pmu: Add const to pmu_events_map." which Andrew Kilroy has replied to. > Ok, this will hopefully change tomorrow after today's Brazilian holiday when I get to process that series, Thanks, - Arnaldo ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 0/3] perf tools: Enable strict JSON parsing @ 2021-10-12 20:15 ` Arnaldo Carvalho de Melo 0 siblings, 0 replies; 36+ messages in thread From: Arnaldo Carvalho de Melo @ 2021-10-12 20:15 UTC (permalink / raw) To: James Clark, Arnaldo Carvalho de Melo, kajoljain Cc: john.garry, ak, linux-perf-users, Nick.Forrington, Andrew.Kilroy, Will Deacon, Mathieu Poirier, Leo Yan, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim, linux-arm-kernel, linux-kernel On October 12, 2021 10:30:51 AM GMT-03:00, James Clark <james.clark@arm.com> wrote: > > >On 08/10/2021 20:00, Arnaldo Carvalho de Melo wrote: >> Em Fri, Oct 08, 2021 at 04:56:55PM +0530, kajoljain escreveu: >>> >>> >>> On 10/8/21 3:32 PM, James Clark wrote: >>>> >>>> >>>> On 08/10/2021 08:43, kajoljain wrote: >>>>> >>>>> >>>>> On 10/7/21 4:35 PM, James Clark wrote: >>>>>> After a discussion on "[PATCH 1/4] perf vendor events: Syntax corrections in Neoverse N1 json", >>>>>> John Garry suggested that we can just modify the parser to make it more strict. Hopefully this will >>>>>> remove the need to apply any future JSON comma fixup commits. >>>>>> >>>>>> Applies on top of "[PATCH v2 1/3] perf vendor events: Syntax corrections in Neoverse N1 json" on >>>>>> perf/core. >>>>>> >>>>>> Also available at: >>>>>> git clone --branch james-json-parse-fix git@git.gitlab.arm.com:linux-arm/linux-jc.git >>>>> >>>>> Hi James, >>>>> Do we have any dependency patches on top of this patch series. I am >>>>> reviewing and testing it, but in both powerpc and x86 system I am >>>>> getting build issue. Not sure if I am missing something> >>>>> I am trying your changes on top of upstream perf. >>>>> >>>>> pmu-events/arch/test/test_soc/sys/uncore.json: json error Invalid >>>>> character inside JSON string >>>> >>>> Hi Kajol, >>>> >>>> A trailing comma was fixed in this file 3 weeks ago at b8b350a. Can you >>>> confirm if you have updated to get this commit on perf core? >>>> >>>> Alternately you could pull from my branch above which is up to date enough >>>> to include it. >>> >>> Hi James, >>> Thanks for pointing it. Not getting build issue now. >>>> >>>> The file is in pmu-events/arch/test/ so I would expect it to fail on all platforms. >>>> >>>>> make[3]: *** [pmu-events/Build:18: pmu-events/pmu-events.c] Error 1 >>>>> make[3]: *** Deleting file 'pmu-events/pmu-events.c' >>>>> make[2]: *** [Makefile.perf:667: pmu-events/pmu-events-in.o] Error 2 >>>>> make[2]: *** Waiting for unfinished jobs.... >>>>> make[1]: *** [Makefile.perf:238: sub-make] Error 2 >>>>> make: *** [Makefile:70: all] Error 2 >>>>> >>>>> Also, Is it possible to add line number along with file name while >>>>> showing this error `json error Invalid character inside JSON string`. >>>>> It might make it easy to fix. >>>> >>>> I can add a character number with the following fix if you think that would >>>> be good enough? A line number might be a bigger change and involve keeping >>>> track of newline characters. >>> >>> Sure. I think then we can skip this change. Not sure if character >>> number will be helpful. >>> >>> Patch-set looks good to me. >>> >>> Reviewed-by Kajol Jain<kjain@linux.ibm.com> >> >> Applied ok as-is to my perf/core branch, applied and added your >> Reviewed-by, thanks. >> > >Thanks Arnaldo. This does mean that the arm64 build will fail until >"[PATCH v2 1/3] perf vendor events: Syntax corrections in Neoverse N1 json" is >applied. I think there is also an arm64 build issue with "[PATCH 02/21] perf >pmu: Add const to pmu_events_map." which Andrew Kilroy has replied to. > Ok, this will hopefully change tomorrow after today's Brazilian holiday when I get to process that series, Thanks, - Arnaldo _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 0/3] perf tools: Enable strict JSON parsing 2021-10-12 13:30 ` James Clark @ 2021-10-13 16:57 ` Arnaldo Carvalho de Melo -1 siblings, 0 replies; 36+ messages in thread From: Arnaldo Carvalho de Melo @ 2021-10-13 16:57 UTC (permalink / raw) To: James Clark Cc: kajoljain, john.garry, ak, linux-perf-users, Nick.Forrington, Andrew.Kilroy, Will Deacon, Mathieu Poirier, Leo Yan, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim, linux-arm-kernel, linux-kernel Em Tue, Oct 12, 2021 at 02:30:51PM +0100, James Clark escreveu: > On 08/10/2021 20:00, Arnaldo Carvalho de Melo wrote: > > Em Fri, Oct 08, 2021 at 04:56:55PM +0530, kajoljain escreveu: > >> On 10/8/21 3:32 PM, James Clark wrote: > >>> On 08/10/2021 08:43, kajoljain wrote: > >> Sure. I think then we can skip this change. Not sure if character > >> number will be helpful. > >> Patch-set looks good to me. > >> Reviewed-by Kajol Jain<kjain@linux.ibm.com> > > Applied ok as-is to my perf/core branch, applied and added your > > Reviewed-by, thanks. > Thanks Arnaldo. This does mean that the arm64 build will fail until > "[PATCH v2 1/3] perf vendor events: Syntax corrections in Neoverse N1 json" is > applied. I think there is also an arm64 build issue with "[PATCH 02/21] perf > pmu: Add const to pmu_events_map." which Andrew Kilroy has replied to. Its all in: 19f8408a634c9515 (HEAD -> perf/core) perf intel-pt: Add support for PERF_RECORD_AUX_OUTPUT_HW_ID 69125e749c006b4f perf tools: Add support for PERF_RECORD_AUX_OUTPUT_HW_ID 1b1d9560a61f1e4e perf vendor events arm64: Categorise the Neoverse V1 counters abe8733bc3575869 perf vendor events arm64: Add new armv8 pmu events d211e9e76a466cce perf vendor events: Syntax corrections in Neoverse N1 json c067335fcbfc67c3 (quaco/perf/core, acme/tmp.perf/core) perf metric: Allow modifiers on metrics. acf2cb9cf242e9ab perf parse-events: Identify broken modifiers fb193eca0ae8ddae perf metric: Switch fprintf() to pr_err() fb8c3a06943cc3c7 perf metrics: Modify setup and deduplication 4965bb2e71371d5f perf expr: Add subset utility c1d7cd1b36fce16b perf metric: Encode and use metric-id as qualifier 3743f880b3856971 perf parse-events: Allow config on kernel PMU events 844f07a5ddcd46c5 perf parse-events: Add new "metric-id" term e68f07424b8b3f00 perf parse-events: Add const to evsel name ace154d9f5dc3331 perf metric: Simplify metric_refs calculation eeffd53b41dc7077 perf metric: Document the internal 'struct metric' 9aa64400defa07fb perf metric: Comment data structures 353ce4ed869635c7 perf metric: Modify resolution and recursion check 0bffecb0ac304bb2 perf metric: Only add a referenced metric once 937323c22db4cb1e perf metric: Add metric new and free 176b9da84871449d perf metric: Add documentation and rename a variable. cc6803c12cef80f1 perf metric: Move runtime value to the expr context 9610bca8f117d963 perf pmu: Make pmu_event tables const 95ed79343835656d perf pmu: Make pmu_sys_event_tables const 05335f28549c7cc5 perf pmu: Add const to pmu_events_map. cac98c8aca3c7dd2 tools lib: Adopt list_sort from the kernel sources f792cf8a094eac29 perf kmem: Improve man page for record options eda1a84cb4e93759 perf tools: Enable strict JSON parsing 21813684e46df1c9 perf tools: Make the JSON parser more conformant when in strict mode 08f3e0873ac20344 perf vendor-events: Fix all remaining invalid JSON files - Arnaldo ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 0/3] perf tools: Enable strict JSON parsing @ 2021-10-13 16:57 ` Arnaldo Carvalho de Melo 0 siblings, 0 replies; 36+ messages in thread From: Arnaldo Carvalho de Melo @ 2021-10-13 16:57 UTC (permalink / raw) To: James Clark Cc: kajoljain, john.garry, ak, linux-perf-users, Nick.Forrington, Andrew.Kilroy, Will Deacon, Mathieu Poirier, Leo Yan, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim, linux-arm-kernel, linux-kernel Em Tue, Oct 12, 2021 at 02:30:51PM +0100, James Clark escreveu: > On 08/10/2021 20:00, Arnaldo Carvalho de Melo wrote: > > Em Fri, Oct 08, 2021 at 04:56:55PM +0530, kajoljain escreveu: > >> On 10/8/21 3:32 PM, James Clark wrote: > >>> On 08/10/2021 08:43, kajoljain wrote: > >> Sure. I think then we can skip this change. Not sure if character > >> number will be helpful. > >> Patch-set looks good to me. > >> Reviewed-by Kajol Jain<kjain@linux.ibm.com> > > Applied ok as-is to my perf/core branch, applied and added your > > Reviewed-by, thanks. > Thanks Arnaldo. This does mean that the arm64 build will fail until > "[PATCH v2 1/3] perf vendor events: Syntax corrections in Neoverse N1 json" is > applied. I think there is also an arm64 build issue with "[PATCH 02/21] perf > pmu: Add const to pmu_events_map." which Andrew Kilroy has replied to. Its all in: 19f8408a634c9515 (HEAD -> perf/core) perf intel-pt: Add support for PERF_RECORD_AUX_OUTPUT_HW_ID 69125e749c006b4f perf tools: Add support for PERF_RECORD_AUX_OUTPUT_HW_ID 1b1d9560a61f1e4e perf vendor events arm64: Categorise the Neoverse V1 counters abe8733bc3575869 perf vendor events arm64: Add new armv8 pmu events d211e9e76a466cce perf vendor events: Syntax corrections in Neoverse N1 json c067335fcbfc67c3 (quaco/perf/core, acme/tmp.perf/core) perf metric: Allow modifiers on metrics. acf2cb9cf242e9ab perf parse-events: Identify broken modifiers fb193eca0ae8ddae perf metric: Switch fprintf() to pr_err() fb8c3a06943cc3c7 perf metrics: Modify setup and deduplication 4965bb2e71371d5f perf expr: Add subset utility c1d7cd1b36fce16b perf metric: Encode and use metric-id as qualifier 3743f880b3856971 perf parse-events: Allow config on kernel PMU events 844f07a5ddcd46c5 perf parse-events: Add new "metric-id" term e68f07424b8b3f00 perf parse-events: Add const to evsel name ace154d9f5dc3331 perf metric: Simplify metric_refs calculation eeffd53b41dc7077 perf metric: Document the internal 'struct metric' 9aa64400defa07fb perf metric: Comment data structures 353ce4ed869635c7 perf metric: Modify resolution and recursion check 0bffecb0ac304bb2 perf metric: Only add a referenced metric once 937323c22db4cb1e perf metric: Add metric new and free 176b9da84871449d perf metric: Add documentation and rename a variable. cc6803c12cef80f1 perf metric: Move runtime value to the expr context 9610bca8f117d963 perf pmu: Make pmu_event tables const 95ed79343835656d perf pmu: Make pmu_sys_event_tables const 05335f28549c7cc5 perf pmu: Add const to pmu_events_map. cac98c8aca3c7dd2 tools lib: Adopt list_sort from the kernel sources f792cf8a094eac29 perf kmem: Improve man page for record options eda1a84cb4e93759 perf tools: Enable strict JSON parsing 21813684e46df1c9 perf tools: Make the JSON parser more conformant when in strict mode 08f3e0873ac20344 perf vendor-events: Fix all remaining invalid JSON files - Arnaldo _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 36+ messages in thread
end of thread, other threads:[~2021-10-13 16:58 UTC | newest] Thread overview: 36+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-10-07 11:05 [PATCH 0/3] perf tools: Enable strict JSON parsing James Clark 2021-10-07 11:05 ` James Clark 2021-10-07 11:05 ` [PATCH 1/3] perf vendor-events: Fix all remaining invalid JSON files James Clark 2021-10-07 11:05 ` James Clark 2021-10-07 17:22 ` John Garry 2021-10-07 17:22 ` John Garry 2021-10-07 11:05 ` [PATCH 2/3] perf tools: Make the JSON parser more conformant when in strict mode James Clark 2021-10-07 11:05 ` James Clark 2021-10-07 17:52 ` Jiri Olsa 2021-10-07 17:52 ` Jiri Olsa 2021-10-08 10:08 ` James Clark 2021-10-08 10:08 ` James Clark 2021-10-08 13:12 ` Jiri Olsa 2021-10-08 13:12 ` Jiri Olsa 2021-10-08 18:56 ` Arnaldo Carvalho de Melo 2021-10-08 18:56 ` Arnaldo Carvalho de Melo 2021-10-08 19:01 ` Arnaldo Carvalho de Melo 2021-10-08 19:01 ` Arnaldo Carvalho de Melo 2021-10-07 11:05 ` [PATCH 3/3] perf tools: Enable strict JSON parsing James Clark 2021-10-07 11:05 ` James Clark 2021-10-07 23:51 ` [PATCH 0/3] " Andi Kleen 2021-10-07 23:51 ` Andi Kleen 2021-10-08 7:43 ` kajoljain 2021-10-08 7:43 ` kajoljain 2021-10-08 10:02 ` James Clark 2021-10-08 10:02 ` James Clark 2021-10-08 11:26 ` kajoljain 2021-10-08 11:26 ` kajoljain 2021-10-08 19:00 ` Arnaldo Carvalho de Melo 2021-10-08 19:00 ` Arnaldo Carvalho de Melo 2021-10-12 13:30 ` James Clark 2021-10-12 13:30 ` James Clark 2021-10-12 20:15 ` Arnaldo Carvalho de Melo 2021-10-12 20:15 ` Arnaldo Carvalho de Melo 2021-10-13 16:57 ` Arnaldo Carvalho de Melo 2021-10-13 16:57 ` Arnaldo Carvalho de Melo
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.