[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2 01/22] x86/include/asm/intel-txt.h: constants and accessors for TXT registers and heap



On Tue, Jun 03, 2025 at 09:06:53AM +0200, Jan Beulich wrote:
> On 03.06.2025 00:00, Sergii Dmytruk wrote:
> > On Mon, Jun 02, 2025 at 09:17:37AM +0200, Jan Beulich wrote:
> >> On 23.05.2025 21:51, Sergii Dmytruk wrote:
> >>> On Wed, May 21, 2025 at 05:19:57PM +0200, Jan Beulich wrote:
> >>>>> +static inline uint64_t txt_bios_data_size(void *heap)
> >>>>
> >>>> Here, below, and in general: Please try to have code be const-correct, 
> >>>> i.e.
> >>>> use pointers-to-const wherever applicable.
> >>>
> >>> I assume this doesn't apply to functions returning `void *`.  The
> >>> approach used in libc is to accept pointers-to-const but then cast the
> >>> constness away for the return value, but this header isn't a widely-used
> >>> code.
> >>
> >> Which is, from all I know, bad practice not only by my own view.
> >
> > I actually ended up doing that to have const-correctness in v3.  In the
> > absence of function overloads the casts have to be somewhere, can put
> > them in the calling code instead.
>
> Casts of which kind? For context: There shouldn't be any casting away of
> const-ness (or volatile-ness, for the sake of completeness).
>
> Jan

Casting away const-ness inside of functions like

    static inline void *txt_bios_data_start(const void *heap)

If a function accepts a const pointer and returns it, this turns a
non-const incoming pointer into a const one.  Without duplicating the
code (either having const and non-const versions or repeating code in
other ways), nothing can be made const cleanly in here including
*_size() functions because they call *_start() functions:

    static inline uint64_t txt_os_mle_data_size(const void *heap)
    {
        return *((const uint64_t *)(txt_bios_data_start(heap) +
                                                     // ^^^^ -- const
                                    txt_bios_data_size(heap))) -
               sizeof(uint64_t);
    }

Regards



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.