Code review comment for lp:~hipl-core/hipl/libhip

On 27/02/12 11:46, Stefan Götz wrote:
> Hi Xin,
>
> your understanding is quite correct. It is simply project policy to make all data as 'const' as possible because it helps to detect bugs. For example:
>
> void foo(list_t *list) {
> if (list = NULL) {
> return;
> } else {
> // do stuff with list
> }
> }
>
> cannot go unnoticed when foo is declared as 'void foo(list_t *const list)'
>
> So you're right: such const qualifiers are not *necessary*. But the hipl team still considers them a very good idea.
>
> Cheers,
> Stefan
>
>

Hi Stefan,

Thanks for this explanation, so we are using const in this case for the
function author to avoid potential bugs and it is also helpful for
others to understand the code. It is a good practice, I will follow this
style.

I have one suggestion: for the function declaration in the header file,
I suggest that we can eliminate this kind of const because it makes no
sense for the API caller. For instance:

In linkedlist.h
void hip_ll_del_by_ptr(struct hip_ll *linkedlist, const void *ptr,
                        free_elem_fn free_element);

In linkedlist.c
void hip_ll_del_by_ptr(struct hip_ll *const linkedlist, const void
*const ptr,
                        free_elem_fn free_element) {
     ... ....
}

Compiler will treat them as the same function. I am also thinking if
there is any compiler option to enforce this const by default, then
perhaps we don't need to type it everywhere :)

Xin