Received-SPF: pass (google.com: domain of linux-kernel+bounces-14148-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Date: Tue, 2 Jan 2024 10:32:57 +0200
From: Leon Romanovsky <leon@kernel.org>
To: Chengchang Tang <tangchengchang@huawei.com>
Cc: Stephen Hemminger <stephen@networkplumber.org>,
	Junxian Huang <huangjunxian6@hisilicon.com>, jgg@ziepe.ca,
	dsahern@gmail.com, netdev@vger.kernel.org,
	linux-rdma@vger.kernel.org, linuxarm@huawei.com,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH iproute2-rc 1/2] rdma: Fix core dump when pretty is used
Message-ID: <20240102083257.GB6361@unreal>
References: <20231229065241.554726-1-huangjunxian6@hisilicon.com>
 <20231229065241.554726-2-huangjunxian6@hisilicon.com>
 <20231229092129.25a526c4@hermes.local>
 <30d8c237-953a-8794-9baa-e21b31d4d88c@huawei.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <30d8c237-953a-8794-9baa-e21b31d4d88c@huawei.com>

On Tue, Jan 02, 2024 at 03:44:29PM +0800, Chengchang Tang wrote:
> 
> 
> On 2023/12/30 1:21, Stephen Hemminger wrote:
> > On Fri, 29 Dec 2023 14:52:40 +0800
> > Junxian Huang <huangjunxian6@hisilicon.com> wrote:
> > 
> > > From: Chengchang Tang <tangchengchang@huawei.com>
> > > 
> > > There will be a core dump when pretty is used as the JSON object
> > > hasn't been opened and closed properly.
> > > 
> > > Before:
> > > $ rdma res show qp -jp -dd
> > > [ {
> > >      "ifindex": 1,
> > >      "ifname": "hns_1",
> > >      "port": 1,
> > >      "lqpn": 1,
> > >      "type": "GSI",
> > >      "state": "RTS",
> > >      "sq-psn": 0,
> > >      "comm": "ib_core"
> > > },
> > > "drv_sq_wqe_cnt": 128,
> > > "drv_sq_max_gs": 2,
> > > "drv_rq_wqe_cnt": 512,
> > > "drv_rq_max_gs": 1,
> > > rdma: json_writer.c:130: jsonw_end: Assertion `self->depth > 0' failed.
> > > Aborted (core dumped)
> > > 
> > > After:
> > > $ rdma res show qp -jp -dd
> > > [ {
> > >          "ifindex": 2,
> > >          "ifname": "hns_2",
> > >          "port": 1,
> > >          "lqpn": 1,
> > >          "type": "GSI",
> > >          "state": "RTS",
> > >          "sq-psn": 0,
> > >          "comm": "ib_core",{
> > >              "drv_sq_wqe_cnt": 128,
> > >              "drv_sq_max_gs": 2,
> > >              "drv_rq_wqe_cnt": 512,
> > >              "drv_rq_max_gs": 1,
> > >              "drv_ext_sge_sge_cnt": 256
> > >          }
> > >      } ]
> > > 
> > > Fixes: 331152752a97 ("rdma: print driver resource attributes")
> > > Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
> > > Signed-off-by: Junxian Huang <huangjunxian6@hisilicon.com>
> > This code in rdma seems to be miking json and newline functionality
> > which creates bug traps.
> > 
> > Also the json should have same effective output in pretty and non-pretty mode.
> > It looks like since pretty mode add extra object layer, the nesting of {} would be
> > different.
> > 
> > The conversion to json_print() was done but it isn't using same conventions
> > as ip or tc.
> > 
> > The correct fix needs to go deeper and hit other things.
> > 
> 
> Hi, Stephen,
> 
> The root cause of this issue is that close_json_object() is being called in
> newline_indent(), resulting in a mismatch
> of {}.
> 
> When fixing this problem, I was unsure why a newline() is needed in pretty
> mode, so I simply kept this logic and
> solved the issue of open_json_object() and close_json_object() not matching.
> However, If the output of pretty mode
> and not-pretty mode should be the same, then this problem can be resolved by
> deleting this newline_indent().

Stephen didn't say that output of pretty and not-pretty should be the
same, but he said that JSON logic should be the same.

Thanks

> 
> I believe the original developer may not have realized that
> close_json_object() is being called in newline(), which leads
> to this problem. To improve the code's readability, I would try to strip out
> close_json_obejct() from newline().
> 
> Thanks,
> Chengchang Tang
>