Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20210715115856.11304-1-song.bao.hua@hisilicon.com>
 <20210715115856.11304-5-song.bao.hua@hisilicon.com> <YPAlg5c8uDSZ2WvZ@smile.fi.intel.com>
 <YPCe+f3GPDUuvwnW@yury-ThinkPad>
In-Reply-To: <YPCe+f3GPDUuvwnW@yury-ThinkPad>
From:   Andy Shevchenko <andy.shevchenko@gmail.com>
Date:   Fri, 16 Jul 2021 00:32:45 +0300
Message-ID: <CAHp75VcoNPiKDaZzTVr3unV3F5u+LQwAjy1hKSq0WUw_tB6uAw@mail.gmail.com>
Subject: Re: [PATCH v7 4/4] lib: test_bitmap: add bitmap_print_to_buf test cases
To:     Yury Norov <yury.norov@gmail.com>
Cc:     Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
        Barry Song <song.bao.hua@hisilicon.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Andrew Morton <akpm@linux-foundation.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Dave Hansen <dave.hansen@intel.com>,
        Rasmus Villemoes <linux@rasmusvillemoes.dk>,
        "Rafael J. Wysocki" <rafael@kernel.org>,
        Randy Dunlap <rdunlap@infradead.org>,
        Alexander Gordeev <agordeev@linux.ibm.com>,
        Stefano Brivio <sbrivio@redhat.com>,
        "Ma, Jianpeng" <jianpeng.ma@intel.com>,
        Valentin Schneider <valentin.schneider@arm.com>,
        "Peter Zijlstra (Intel)" <peterz@infradead.org>,
        Daniel Bristot de Oliveira <bristot@redhat.com>,
        Guodong Xu <guodong.xu@linaro.org>, tangchengchang@huawei.com,
        "Zengtao (B)" <prime.zeng@hisilicon.com>,
        yangyicong <yangyicong@huawei.com>, tim.c.chen@linux.intel.com,
        Linuxarm <linuxarm@huawei.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Thu, Jul 15, 2021 at 11:48 PM Yury Norov <yury.norov@gmail.com> wrote:
> On Thu, Jul 15, 2021 at 03:09:39PM +0300, Andy Shevchenko wrote:
> > On Thu, Jul 15, 2021 at 11:58:56PM +1200, Barry Song wrote:
> > > The added test items cover both cases where bitmap buf of the printed
> > > result is greater than and less than 4KB.
> > > And it also covers the case where offset for bitmap_print_to_buf is
> > > non-zero which will happen when printed buf is larger than one page
> > > in sysfs bin_attribute.
> >
> > More test cases is always a good thing, thanks!
>
> Generally yes. But in this case... I believe, Barry didn't write that
> huge line below by himself. Most probably he copy-pasted the output of
> his bitmap_print_buf() into the test. If so, this code tests nothing,
> and just enforces current behavior of snprintf.

I'm not sure I got what you are telling me. The big line is to test
strings that are bigger than 4k.

...

> > > +static const char large_list[] __initconst = /* more than 4KB */
> > > +   "0,4,8,12,16,20,24,28,32-33,36-37,40-41,44-45,48-49,52-53,56-57,60-61,64,68,72,76,80,84,88,92,96-97,100-101,104-1"
> > > +   "05,108-109,112-113,116-117,120-121,124-125,128,132,136,140,144,148,152,156,160-161,164-165,168-169,172-173,176-1"
> > > +   "77,180-181,184-185,188-189,192,196,200,204,208,212,216,220,224-225,228-229,232-233,236-237,240-241,244-245,248-2"
>
> I don't like this behavior of the code: each individual line is not a
> valid bitmap_list. I would prefer to split original bitmap and print
> list representation of parts in a compatible format; considering a
> receiving part of this splitting machinery.

I agree that split is not the best here, but after all it's only 1
line and this is on purpose.

-- 
With Best Regards,
Andy Shevchenko