• Yury Norov's avatar
    bitmap: new bitmap_copy_safe and bitmap_{from,to}_arr32 · c724f193
    Yury Norov authored
    This patchset replaces bitmap_{to,from}_u32array with more simple and
    standard looking copy-like functions.
    
    bitmap_from_u32array() takes 4 arguments (bitmap_to_u32array is similar):
     - unsigned long *bitmap, which is destination;
     - unsigned int nbits, the length of destination bitmap, in bits;
     - const u32 *buf, the source; and
     - unsigned int nwords, the length of source buffer in ints.
    
    In description to the function it is detailed like:
    * copy min(nbits, 32*nwords) bits from @buf to @bitmap, remaining
    * bits between nword and nbits in @bitmap (if any) are cleared.
    
    Having two size arguments looks unneeded and potentially dangerous.
    
    It is unneeded because normally user of copy-like function should take
    care of the size of destination and make it big enough to fit source
    data.
    
    And it is dangerous because function may hide possible error if user
    doesn't provide big enough bitmap, and data becomes silently dropped.
    
    That's why all copy-like functions have 1 argument for size of copying
    data, and I don't see any reason to make bitmap_from_u32array()
    different.
    
    One exception that comes in mind is strncpy() which also provides size
    of destination in arguments, but it's strongly argued by the possibility
    of taking broken strings in source.  This is not the case of
    bitmap_{from,to}_u32array().
    
    There is no many real users of bitmap_{from,to}_u32array(), and they all
    very clearly provide size of destination matched with the size of
    source, so additional functionality is not used in fact. Like this:
    bitmap_from_u32array(to->link_modes.supported,
    		__ETHTOOL_LINK_MODE_MASK_NBITS,
    		link_usettings.link_modes.supported,
    		__ETHTOOL_LINK_MODE_MASK_NU32);
    Where:
    #define __ETHTOOL_LINK_MODE_MASK_NU32 \
    	DIV_ROUND_UP(__ETHTOOL_LINK_MODE_MASK_NBITS, 32)
    
    In this patch, bitmap_copy_safe and bitmap_{from,to}_arr32 are introduced.
    
    'Safe' in bitmap_copy_safe() stands for clearing unused bits in bitmap
    beyond last bit till the end of last word. It is useful for hardening
    API when bitmap is assumed to be exposed to userspace.
    
    bitmap_{from,to}_arr32 functions are replacements for
    bitmap_{from,to}_u32array. They don't take unneeded nwords argument, and
    so simpler in implementation and understanding.
    
    This patch suggests optimization for 32-bit systems - aliasing
    bitmap_{from,to}_arr32 to bitmap_copy_safe.
    
    Other possible optimization is aliasing 64-bit LE bitmap_{from,to}_arr32 to
    more generic function(s). But I didn't end up with the function that would
    be helpful by itself, and can be used to alias 64-bit LE
    bitmap_{from,to}_arr32, like bitmap_copy_safe() does. So I preferred to
    leave things as is.
    
    The following patch switches kernel to new API and introduces test for it.
    
    Discussion is here: https://lkml.org/lkml/2017/11/15/592
    
    [ynorov@caviumnetworks.com: rename bitmap_copy_safe to bitmap_copy_clear_tail]
      Link: http://lkml.kernel.org/r/20180201172508.5739-3-ynorov@caviumnetworks.com
    Link: http://lkml.kernel.org/r/20171228150019.27953-1-ynorov@caviumnetworks.comSigned-off-by: default avatarYury Norov <ynorov@caviumnetworks.com>
    Cc: Ben Hutchings <ben@decadent.org.uk>
    Cc: David Decotigny <decot@googlers.com>,
    Cc: David S. Miller <davem@davemloft.net>,
    Cc: Geert Uytterhoeven <geert@linux-m68k.org>
    Cc: Matthew Wilcox <mawilcox@microsoft.com>
    Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
    Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
    Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
    c724f193
bitmap.c 37.9 KB