Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <6c8e4104-2239-a188-649d-585f059cabdd@intel.com>
Date:   Fri, 8 Jul 2022 14:13:14 +0200
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Firefox/91.0 Thunderbird/91.10.0
Subject: Re: [PATCH 1/2] lib/string_helpers: Introduce strsplit_u32()
Content-Language: en-US
To:     Andy Shevchenko <andy.shevchenko@gmail.com>
CC:     Andy Shevchenko <andy@kernel.org>, Mark Brown <broonie@kernel.org>,
        "ALSA Development Mailing List" <alsa-devel@alsa-project.org>,
        Takashi Iwai <tiwai@suse.com>,
        Jaroslav Kysela <perex@perex.cz>,
        <amadeuszx.slawinski@linux.intel.com>,
        Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>,
        Hans de Goede <hdegoede@redhat.com>,
        =?UTF-8?Q?P=c3=a9ter_Ujfalusi?= <peter.ujfalusi@linux.intel.com>,
        "Ranjani Sridharan" <ranjani.sridharan@linux.intel.com>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Liam Girdwood <lgirdwood@gmail.com>,
        "Kai Vehmanen" <kai.vehmanen@linux.intel.com>,
        Bard Liao <yung-chuan.liao@linux.intel.com>
References: <20220707091301.1282291-1-cezary.rojewski@intel.com>
 <CAHp75VceKBoxXVPP4dRYb8LQqHMMDHFp6-E2iuZ-h2RTK8PWQQ@mail.gmail.com>
 <e0c7d254-ace3-625c-cc83-52ca0b45e9fc@intel.com>
 <CAHp75VckU2ZraLJ-frjWXjUu9pFW+-XmWgTbYqUXOUNAD-1HGA@mail.gmail.com>
From:   Cezary Rojewski <cezary.rojewski@intel.com>
In-Reply-To: <CAHp75VckU2ZraLJ-frjWXjUu9pFW+-XmWgTbYqUXOUNAD-1HGA@mail.gmail.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?MXp1RHdmRndkVW5LSnFlTWVsUDdtNksyclBJK05lLzNhMGlCa3ZJZTRwRUc2?=
 =?utf-8?B?SDU2bXZjdVhhbGpiY1pDYWpMQlk2bFdBcGZvdmd5NkQ2cTVnTXdReWlXWVYw?=
 =?utf-8?B?SHA5WHVuR1hjMkd6MTdQdTVBdHRMWjFQby9BVU1BUG05MitwRjM0Y1BsUmJj?=
 =?utf-8?B?TXlDQzV3dmRUeTNkVDRsd0Z5TDl1SGlVT3pEbFloNi9DNVMxN1ExQnRJSVhT?=
 =?utf-8?B?Z3I1Z0VpY2RsRWRQSUk0cXAvU3k5Z0ZjcDhUQmVqQzZjbmNRVUtnZVA2MlB3?=
 =?utf-8?B?MXJPdmdQRVMzb3FXMnVXdVpJTGFIcVZWM09McTlCVVk4VU41aUcyQjVSZSth?=
 =?utf-8?B?L25RN3h3UHpTMjFBNmY3TkJwSEZFd3hSWExoREJiRUt3L2t3NWZEUXNrQWEy?=
 =?utf-8?B?enVEcHc3WUJ3T2ZWb090cVI0cmFhT3dpdWV1SSt6a3pCaDdXQ1ZiVmZSSkVY?=
 =?utf-8?B?L1FheWhaSXBaVEJuT3FYdk1OL2pNUVAwU2hCVm1HbHRMYU5SZ2pJTDBWNnJW?=
 =?utf-8?B?YVI0cStzaTZvSUIvUEZwdk9ZTUIzTHYvNUhSY0NYQ0RXME8vbXJFcURXUERx?=
 =?utf-8?B?b1ZsK3ZTWmNNSEhoODloR1c3c0lUYUtKL3RtbXkxcUhRcVNtMHBsMFc3NnN2?=
 =?utf-8?B?S09WeTZNak5nZk1vc0FYWW94ajh4cElRQlA5amhvL05NL084cTRvQk5jMFM5?=
 =?utf-8?B?a2NHVGxnS2plWWZEMnd4RVNWbFRTZHpQdDV2WEM5SXk0aVA0eE5zSzg1Vy9a?=
 =?utf-8?B?WGdYbFFUaGZ2MCtDaWc2bnIyOXliYWlvSTRndVR5SE5pcDd4UzNFV01MYU8w?=
 =?utf-8?B?bElyMGh5WjR2by92Mm1uZ0pBeVpwV3ptT1JEVkgzTWFHRzVTWXRRNzVHcUJy?=
 =?utf-8?B?UFdTSCt0WW4za25zWGlzKzkwc0VWOFozVnIxZlJpTDJtdFcwNzAxdWtnRnVw?=
 =?utf-8?B?SXVqaE5yYTVLT3ZwK3B2M2tWeUFvM0Q5ckVqeG5xZGZhRHdyT3hKS1E0WU1G?=
 =?utf-8?B?UURzMERMOHZGMGZOZlJCMktxN3hyUjFnNEwzT3NPYy9KRjl1aWhZK2RDYkt6?=
 =?utf-8?B?dFh3bFIvZEZwbzBwbHlOZUZCQkhVQjVrZHpQQzMzQVBKQXdFYTg4YzRPQ2p3?=
 =?utf-8?B?MVlGbURyZFFFdlZJZWFwLzNPSklRYXJWb3R1TTFLRXV5eGNBMU1BTXFmMDQz?=
 =?utf-8?B?dnRaZE5aUXBPZllFUHlmTXVZdHlZMk9kcnJ1MmRBWDVNMkI4d1ZEQU5NT2sw?=
 =?utf-8?B?UEtIbTRFU3VkaDAxdllydVFKTmE5dzVVTTI3K254d25hT1R5aXVXWnVVVWth?=
 =?utf-8?B?VFpGdHRjTHFCMVFBZ1Fpa09jQnBPZEdZVVhPdjdyaldVNzdlaEFQSFdDKzkx?=
 =?utf-8?B?bWZPSjh1VHVLY3FqeGZoc3BtK1VobUZZQ2ZGQmxCR2VmdFFOQUJaUEt0MDAy?=
 =?utf-8?B?a3lDbEsrMFY1YkVCU1g3ZEZWME1UeHhHNHNFY0VGNnhIVzFwaGJPRWtYclZa?=
 =?utf-8?B?VkxpU2FUZFlHQ1U1cTBZY1AyUDNqQ3d1cXVZMjZYVk1XazMzYzdRRXFCTURp?=
 =?utf-8?B?SkdlMWJWZjVKWlRqc3grMG5RbjBkaDU2QzVWWmU4dDUxTFpwYUhIQ2MwOEx4?=
 =?utf-8?B?aHBJR0drZnBzL0loSmNFVEo5QjNZR1ZMM2FwNG9RQ1VDMGR4eUVLMUJaS1B0?=
 =?utf-8?B?T0lxRStLOCswR1VUa0k5dkJRUCtHa0VzL203ZzZ2NEt1N0tncExtQS96MW9l?=
 =?utf-8?B?OVRvTU1BOTRBa3dTanlKZ1ArU1RtV0dCQUJxajBzTmZSSi9tNVJsOXFvNHBE?=
 =?utf-8?B?ZGhReGhMeXhNS1ZzYUdBaUNkQUQyNU4rb1kzeFo2bkJZTG82RVV0VElwekd3?=
 =?utf-8?B?SjByWFBwUGI2UzVHOUZrUHRNK3JKS2svVnNCSGducFZGaEJ1WmgwNWNTUnRD?=
 =?utf-8?B?allDVkhXRHZ5S3F0bWFzeTIwN21mUlZQb2tEZTJETHhkTjZlTXlpWDAzMkVy?=
 =?utf-8?B?Ulpzai83N2pZTk9xMjJDRU96NW5PMlUyMFk1NU9GYnhkMU1xblJFOHNEbVU5?=
 =?utf-8?B?WjZGRk1hL1RtL1RDRmdreGZJUnM4d0RPNzRtTk9ocEhTWG5XL2xVVlFkejdw?=
 =?utf-8?B?R2xoK0tOSDFPdmFxaFZkNW1HTUE3SWk1UUxRTU4vLzBzK21qQ0VSVmcydW1G?=
 =?utf-8?B?Smc9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: dd1c0666-e3a9-4ea8-174c-08da60db40cd
X-MS-Exchange-CrossTenant-AuthSource: DS0PR11MB6375.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 08 Jul 2022 12:13:22.8883
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: mK3vg4NmWDQeZ96eWQcsqMCuOmGkb94S62u5DPt9M6Pd98SVw0duN4+pqUhqG1fOofH+aPY+USm4IQTjdr68IV2hixrtF3sEhs5ndWG0j8E=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR11MB2539
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-7.8 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,
        RCVD_IN_DNSWL_HI,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,
        SPF_NONE,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no
        version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 2022-07-08 1:46 PM, Andy Shevchenko wrote:
> On Fri, Jul 8, 2022 at 1:33 PM Cezary Rojewski
> <cezary.rojewski@intel.com> wrote:

...

>> When I'd written the very first version of this function many months
>> ago, get_options() looked as it does not fulfill our needs. It seems to
>> be true even today: caller needs to know the number of elements in an
>> array upfront.
> 
> Have you read a kernel doc for it? It does return the number of
> elements at the first pass.

Yes, I've checked several parts of it. Perhaps I did miss something but 
simple_strtoull() doc reads: use kstrtox() instead. Thus the 
strsplit_u32() makes use of kstrtox().

>> Also, kstrtox() takes into account '0x' and modifies the
>> base accordingly if that's the case. simple_strtoull() looks as not
>> capable of doing the same thing.
> 
> How come?! It does parse all known prefixes: 0x, 0, +, -.

Hmm.. doc says that it stops at the first non-digit character. Will 
re-check.

>> The goal is to be able to parse input such as:
>>
>> 0x1000003,0,0,0x1000004,0,0
>>
>> into a sequence of 6 uints, filling the *tkns and *num_tkns for the caller.
> 
> Yes. Have you checked the test cases for get_options()?
> 

...

>> avs-driver, which is also part of the ASoC framework has very similar
>> debug-interface. I believe there's no need to duplicate the functions -
>> move them to common code instead.
> 
> Taking the above into account, please try to use get_options() and
> then tell me what's not working with it. If so, we will add test cases
> to get_options() and fix it.

There is a difference:

	// get_options
	int ints[5];

	s = get_options(str, ARRAY_SIZE(ints), ints);

	// strsplit_u32()
	u32 *tkns, num_tkns;

	ret = strsplit_u32(str, delim, &tkns, &num_tkns);

Nothing has been told upfront for in the second case.