On 09/14/2016 04:21 PM, Jeff Janes wrote:
>> I suggest that pageinspect functions are more convenient to use via the
>> get_raw_page interface, that is, instead of reading the buffer
>> themselves, the buffer is handed over from elsewhere and they receive it
>> as bytea. This enables other use cases such as grabbing a page from one
>> server and examining it in another one.
>>
>
> I've never needed to do that, but it does sound like it might be useful.
> But it is also annoying to have to do that when you want to examine a
> current server. Could we use overloading, so that it can be used in either
> way?
>
> For hash_page_items, the 'data' is always a 32 bit integer, isn't it?
> (unlike other pageinspect features, where the data could be any
> user-defined data type). If so, I'd rather see it in plain hex (without
> the spaces, without the trailing zeros)
>
> + /* page type (flags) */
> + if (opaque->hasho_flag & LH_UNUSED_PAGE)
> + stat->type = 'u';
>
> This can never be true, because LH_UNUSED_PAGE is zero. You should make
> this be the fall-through case, and have LH_META_PAGE be explicitly tested
> for.
>
This version adds the overloaded get_raw_page based methods. However,
I'm not verifying the page other than page_id, as hasho_flag can contain
multiple flags in Amit's patches. And, I don't see having different page
ids as a benefit - at least not part of this patch.
We should probably just have one of the method sets, so I'll send a v3
with the set voted for.
I kept the 'data' field as is, for now.
Best regards,
Jesper