linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Chen Gang <gang.chen@asianux.com>
To: David Rientjes <rientjes@google.com>
Cc: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>,
	kosaki.motohiro@gmail.com, riel@redhat.com, hughd@google.com,
	xemul@parallels.com, liwanp@linux.vnet.ibm.com,
	gorcunov@gmail.com, linux-mm@kvack.org,
	akpm@linux-foundation.org
Subject: Re: [PATCH v2] mm/shmem.c: check the return value of mpol_to_str()
Date: Wed, 18 Sep 2013 09:20:40 +0800	[thread overview]
Message-ID: <5238FFE8.5080008@asianux.com> (raw)
In-Reply-To: <alpine.DEB.2.02.1309171549140.21696@chino.kir.corp.google.com>

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@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  reply	other threads:[~2013-09-18  1:21 UTC|newest]

Thread overview: 61+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-08-20  3:56 [PATCH 0/3] mm: mempolicy: the failure processing about mpol_to_str() Chen Gang
2013-08-20  3:57 ` [PATCH 1/3] mm/mempolicy.c: still fill buffer as full as possible when buffer space is not enough in mpol_to_str() Chen Gang
2013-08-20  3:58   ` [PATCH 2/3] fs/proc/task_mmu.c: check the return value of mpol_to_str() Chen Gang
2013-08-20  3:59     ` [PATCH 3/3] mm/shmem.c: " Chen Gang
2013-08-20  5:30 ` [PATCH 0/3] mm: mempolicy: the failure processing about mpol_to_str() Cyrill Gorcunov
2013-08-20  5:41   ` Chen Gang
2013-08-20  6:47     ` Cyrill Gorcunov
2013-08-20  7:48       ` Chen Gang
2013-08-20  7:51         ` Chen Gang
2013-08-20  8:09           ` Chen Gang
2013-08-20  8:13             ` Chen Gang F T
2013-08-20  8:20               ` Chen Gang
2013-08-20  8:25             ` Cyrill Gorcunov
2013-08-20  8:31               ` Chen Gang
2013-08-21  2:21               ` [PATCH 0/3] mm: shmem: check the return value of mpol_to_str() Chen Gang
2013-08-21  2:22                 ` [PATCH 1/3] fs/proc/task_mmu.c: " Chen Gang
2013-08-21  2:23                   ` [PATCH 2/3] mm/shmem.c: let shmem_show_mpol() return value Chen Gang
2013-08-21  2:24                     ` [PATCH 3/3] mm/shmem.c: check the return value of mpol_to_str() Chen Gang
2013-08-21 22:03                     ` [PATCH 2/3] mm/shmem.c: let shmem_show_mpol() return value Andrew Morton
2013-08-22  0:52                       ` Chen Gang
2013-08-22  1:04                         ` [PATCH] mm/shmem.c: check the return value of mpol_to_str() Chen Gang
2013-09-03  5:32                           ` Chen Gang
2013-09-05  0:24                           ` [PATCH v2] " Chen Gang
2013-09-09 20:30                             ` David Rientjes
2013-09-10  0:47                               ` Chen Gang
2013-09-10  6:43                                 ` David Rientjes
2013-09-10  7:01                                   ` Chen Gang
2013-09-12  0:33                                     ` David Rientjes
2013-09-12  2:19                                       ` KOSAKI Motohiro
2013-09-12  3:13                                         ` Chen Gang
2013-09-13 21:12                                         ` David Rientjes
2013-09-14  2:51                                           ` KOSAKI Motohiro
2013-09-16  3:27                                             ` Chen Gang
2013-09-16 20:13                                               ` David Rientjes
2013-09-17  0:45                                                 ` Chen Gang
2013-09-17 22:51                                                   ` David Rientjes
2013-09-18  1:20                                                     ` Chen Gang [this message]
2013-09-12  3:02                                       ` Chen Gang
2013-09-12 18:19                                         ` KOSAKI Motohiro
2013-09-13  2:23                                           ` Chen Gang
2013-09-13 16:50                                             ` KOSAKI Motohiro
2013-09-16  2:55                                               ` Chen Gang
2013-09-16 16:16                                                 ` KOSAKI Motohiro
2013-09-17  1:10                                                   ` Chen Gang
2013-09-17 22:53                                                     ` David Rientjes
2013-09-18  1:37                                                       ` Chen Gang
2013-09-18 22:17                                                         ` David Rientjes
2013-09-13 21:14                                         ` David Rientjes
2013-09-16  3:17                                           ` Chen Gang
2013-09-25  2:58                             ` [patch] mm, mempolicy: make mpol_to_str robust and always succeed David Rientjes
2013-09-25  3:11                               ` Dave Jones
2013-09-25  3:18                                 ` David Rientjes
2013-09-25  3:25                                   ` Dave Jones
2013-09-25 17:58                                     ` David Rientjes
2013-09-25 21:30                                       ` Andrew Morton
2013-09-25 22:06                                         ` David Rientjes
2013-08-21  5:31                 ` [PATCH 0/3] mm: shmem: check the return value of mpol_to_str() Cyrill Gorcunov
2013-08-21  5:48                   ` Chen Gang
2013-09-19  0:31 [PATCH v2] mm/shmem.c: " Chen,Gang( 陈刚)
     [not found] <20130919003142.B72EC1840296@intranet.asianux.com>
2013-09-23 21:46 ` David Rientjes
2013-09-24  2:28   ` Chen Gang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5238FFE8.5080008@asianux.com \
    --to=gang.chen@asianux.com \
    --cc=akpm@linux-foundation.org \
    --cc=gorcunov@gmail.com \
    --cc=hughd@google.com \
    --cc=kosaki.motohiro@gmail.com \
    --cc=kosaki.motohiro@jp.fujitsu.com \
    --cc=linux-mm@kvack.org \
    --cc=liwanp@linux.vnet.ibm.com \
    --cc=riel@redhat.com \
    --cc=rientjes@google.com \
    --cc=xemul@parallels.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox