On Fri, Sep 26, 2025 at 08:50:10PM +0900, Michael Paquier wrote:
> On Fri, Sep 26, 2025 at 03:35:54PM +0800, Chao Li wrote:
>>> I noticed that many functions take "Datum *" parameters while they don't
>>> update the data. So I created this patch to change "Datum *" to "const
>>> Datum *" wherever possible, which should improve type safety and make the
>>> interfaces clearer about their intent, also helps the compiler catch
>>> accidental modifications.
>
> I have not looked at the patch in details and we need a careful
> case-by-case review, but being more protective with the Datums that
> travel across the stack may be a good idea in the long-term depending
> on the code path dealt with, so I like the initiative you are taking
> here.
I have mixed feelings about this patch. I have no concrete objections to
the technical content, but some questions come to mind. For example, why
are we only fixing Datum parameters? Are we going to fix other types
later? Also, I'm a bit worried that this will lead to several rounds of
similar patches, which IMHO is arguably unnecessary churn that could lead
to back-patching pain. I personally would be more likely to fix these
sorts of things in passing as part of another patch that touches the same
area of code.
That being said, I won't object if Michael or someone else feels it's
worthwhile. I usually try to add const to new function parameters whenever
possible, for exactly the reasons given as motivation for this patch.
--
nathan