lkml.org 
[lkml]   [2012]   [Jul]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH TRIVIAL] mm: Fix build warning in kmem_cache_create()
    On Mon, 23 Jul 2012, Glauber Costa wrote:

    > >> worth including unconditionally. Furthermore, the size related checks
    > >> certainly make sense and I don't see any harm in having them as well.
    > >
    > > There is a WARN_ON() there and then it returns NULL!!! Crazy. Causes a
    > > NULL pointer dereference later in the caller?
    > >
    >
    > It obviously depends on the caller.

    This is a violation of the calling convention to say the least. This means
    if you have SLAB_PANIC set and accidentally set the name to NULL the
    function will return despite the error and not panic!

    > Although most of the calls to kmem_cache_create are made from static
    > data, we can't assume that. Of course whoever is using static data
    > should do those very same tests from the outside to be safe, but in case
    > they do not, this seems to fall in the category of things that make
    > debugging easier - even if we later on get to a NULL pointer dereference.
    >
    > Your mentioned bias towards minimum code size, however, is totally
    > valid, IMHO. But I doubt those checks would introduce a huge footprint.
    > I would imagine you being much more concerned about being able to wipe
    > out entire subsystems like memcg, which will give you a lot more.

    They are useless checks since any use of the name will also cause a NULL
    pointer dereference. Same is true for interrupt checks. Checks like that
    indicate a deterioration of the code base. People are afraid that
    something goes wrong because they no longer understand the code so they
    build a embroidery around it instead of relying on the already existing
    checks at vital places. The embroidery can be useful for debugging thats
    why I left it in for the CONFIG_DEBUG_VM but certainly should not be
    included in production kernels.




    \
     
     \ /
      Last update: 2012-07-25 18:02    [W:0.027 / U:60.952 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site