Re: [Xen-devel] [PATCH] xen/mm: Fix page_list_* helpers to evaluate all their arguments

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 05.03.16 at 17:52, <andrew.cooper3@xxxxxxxxxx> wrote:
> Doing this reveals that const-correctness of page_list_{next,prev}() is
> suspect, taking a const pointer and returning a non-const one.  It is left
> functioning as it did before, with an explicit typecast to remove constness.

I don't see anything suspect here: Retrieving the next or prev list
element doesn't alter the current one, so the input pointer can
legitimately be const, while the output one obviously shouldn't be.
Or else why don't you similarly consider page_list_{first,last}()
bogus in that same respect?

> +static inline bool_t
> +page_list_empty(const struct page_list_head *head)
> +{
> +    return list_empty(head);
> +}

While I appreciate the conversion to bool_t, to be fully correct you
either need to use !! here, or switch list_empty() to bool_t too.

> +static inline struct page_info *
> +page_list_first(const struct page_list_head *head)
> +{
> +    return list_first_entry(head, struct page_info, list);
> +}
> +static inline struct page_info *
> +page_list_last(const struct page_list_head *head)
> +{
> +    return list_last_entry(head, struct page_info, list);
> +}
> +static inline struct page_info *
> +page_list_next(const struct page_info *page,
> +               const struct page_list_head *head)
> +{
> +    return list_next_entry((struct page_info *)page, list);
> +}
> +static inline struct page_info *
> +page_list_prev(const struct page_info *page,
> +               const struct page_list_head *head)
> +{
> +    return list_prev_entry((struct page_info *)page, list);
> +}

I'd suggest to avoid the explicit casts, by using
list_entry(page->list.next, struct page_info, list) and
list_entry(page->list.prev, struct page_info, list) respectively.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel