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 [email protected]: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
Remove trailing commas. A later commit will make the parser more strict
and these will not be valid anymore.
Signed-off-by: James Clark <[email protected]>
---
.../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
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 <[email protected]>
---
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
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<[email protected]>
> ---
> .../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 <[email protected]>
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 <[email protected]>
> ---
> 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
>
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 <[email protected]>
---
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
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 <[email protected]>
-Andi
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 [email protected]: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(-)
>
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 [email protected]: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
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 <[email protected]>
>> ---
>> 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
>>
>
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 [email protected]: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<[email protected]>
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
>
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 <[email protected]>
> >> ---
> >> 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 <[email protected]>
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
> >>
> >
>
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 <[email protected]>
> > >> ---
> > >> 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 <[email protected]>
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
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 <[email protected]>
> > > >> ---
> > > >> 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 <[email protected]>
>
> So, is this for the whole patchset? b4 picked it just for this message.
Got it from IRC, thanks,
- Arnaldo
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 [email protected]: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<[email protected]>
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
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 [email protected]: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<[email protected]>
>
> 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
On October 12, 2021 10:30:51 AM GMT-03:00, James Clark <[email protected]> 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 [email protected]: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<[email protected]>
>>
>> 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
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<[email protected]>
> > 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