lkml.org 
[lkml]   [2023]   [Nov]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v2] x86: Ensure input to pfn to kaddr() is treated as a 64-bit type
On November 15, 2023 5:42:31 PM EST, Michael Roth <michael.roth@amd.com> wrote:
>On Wed, Nov 15, 2023 at 12:48:58PM -0800, Dave Hansen wrote:
>> On 11/15/23 12:14, Michael Roth wrote:
>> > While it might be argued that the issue is on the caller side, other
>> > archs/macros have taken similar approaches to deal with instances like
>> > this, such as commit e48866647b48 ("ARM: 8396/1: use phys_addr_t in
>> > pfn_to_kaddr()").
>>
>> Gah, I really hope nobody is arguing that for real, or is even thinking
>> about this as a valid argument.
>
>Not that I'm aware, but I did have my own doubts initially, which is
>why I thought it warranted a note in the commit just in case it came up
>from someone else.
>
>>
>> The helper should, well, help the caller. It makes zero sense to me
>> that every single call site would need to know if the argument's type
>> was big enough to hold the _return_ value. This nonsense can only even
>> happen with macros. Type promotion would just do the right thing for
>> any sanely declared actual helper function.
>
>My thought was that it is easier to expect developers to know the pitfalls
>of bit-field types, since it is universally applicable to all C code,
>whereas expecting developers to anticipate such issues when writing similar
>macros is potentially harder to enforce/audit and could lead to similar
>issues popping up as things are refactored over time and new macros get
>added that don't take such usages into account.
>
>But neither argument seems to hold up in reality. Experienced developers
>obviously do fall victim to the subtleties of of bit-field types, and
>kernel devs obviously do tend to address these instances in more robust
>ways based on the various pfn-related macros I looked through.
>
>-Mike

Now, if you are doing a cast, you are making the macro unusable for assembly anyway; any reason not to make it an inline function at that point?

\
 
 \ /
  Last update: 2023-11-20 14:01    [W:0.070 / U:0.408 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site