Re: [PATCH v2] mm/shmem.c: check the return value of mpol_to_str()

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

 



On 09/18/2013 06:51 AM, David Rientjes wrote:
> On Tue, 17 Sep 2013, Chen Gang wrote:
> 
>>> Rename mpol_to_str() to __mpol_to_str().  Make a static inline function in 
>>> mempolicy.h named mpol_to_str().  That function does BUILD_BUG_ON(maxlen < 
>>> 64) and then calls __mpol_to_str().
>>>
>>> Modify __mpol_to_str() to store "unknown" when mpol->mode does not match 
>>> any known MPOL_* constant.
>>>
>>
>> Can we be sure 'maxlen' should not be less than 64?  For show_numa_map()
>> in fs/proc/task_mmu.c, it use 50 which is less than 64, is it correct?
>>
> 
> Whatever the max string length is that can be stored by mpol_to_str() 
> preferably rounded to the nearest power of two.
> 

Do you mean: show_numa_map() in "fs/proc/task_mmu.c" also need be
'fixed', what it has done (use 50) is incorrect?


>> Can we be sure that our output contents are always less than 64 bytes?
>> Do we need BUG_ON() instead of all '-ENOSPC' in mpol_to_str()?
>>
> 
> You can determine the maximum string length by looking at the 
> implementation of mpol_to_str().
> 

Can we be sure maximum string will be never changed in future?

>> Hmm... If assume what you said above was always correct: "we are always
>> sure 64 bytes is enough, and 'maxlen' should be never less than 64".
>>
>>   It would be better to use a structure (which has a member "char buf[64]") pointer instead of 'buffer' and 'maxlen'.
>>    (and also still need check 64 memory bondary and '\0' within mpol_to_str).
>>
> 
> That's ridiculous, kernel developers who call mpol_to_str() aren't idiots.
> 

It seems, it is not quite polite. ;-)


Hmm... for extern function, caller has duty to understand interface
precisely, but no duty to understand internal implementation.

So if callee wants caller to know about something, it needs 'express' it
through its' interface, callee can not assume that caller should
understand internal implementation.


> I think at this point it will just be best if I propose a patch and ask 
> for it to be merged into the -mm tree rather than continue this thread.
> 
> 

Hmm... you can try to send a related patch for it, but I am not quite
sure whether can pass reviewers' checking or not.


Thanks.
-- 
Chen Gang

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]