lkml.org 
[lkml]   [2018]   [Sep]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH net-next v6 23/23] net: WireGuard secure network tunnel
    Hi Jason

    I know you have been concentrating on the crypto code, so i'm not
    expecting too many changes at the moment in the network code.

    It would be good to further reduce the number of checkpatch warnings.
    The biggest problem i have at the moment is that all the trivial to
    fix warnings are hiding a few more important warnings. There are a few
    like these which are not obvious to see:

    WARNING: Avoid crashing the kernel - try using WARN_ON & recovery code rather than BUG() or BUG_ON()
    #2984: FILE: drivers/net/wireguard/noise.c:293:
    + BUG_ON(first_len > BLAKE2S_HASH_SIZE || second_len > BLAKE2S_HASH_SIZE ||

    WARNING: Macros with flow control statements should be avoided
    #5471: FILE: drivers/net/wireguard/selftest/allowedips.h:456:
    +#define init_peer(name) do { \
    + name = kzalloc(sizeof(*name), GFP_KERNEL); \
    + if (unlikely(!name)) { \
    + pr_info("allowedips self-test: out of memory\n"); \
    + goto free; \
    + } \
    + kref_init(&name->refcount); \
    + } while (0)


    The namespace pollution also needs to be addresses. You have some
    pretty generic named global symbols. I picked out a few examples from
    objdump

    00002a94 g F .text 00000060 peer_put
    00003484 g F .text 0000004c timers_stop
    00003520 g F .text 00000114 packet_queue_init
    00002640 g F .text 00000034 device_uninit
    000026bc g F .text 00000288 peer_create
    000090d4 g F .text 000001bc ratelimiter_init

    Please make use of a prefix for global symbols, e.g. wg_.

    Andrew

    \
     
     \ /
      Last update: 2018-09-27 03:16    [W:4.949 / U:0.348 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site