linux-kernel - Re: [PATCH 1/10] cxgb3

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <Pine.LNX.4.61.0611181757010.5252@yvahk01.tjqt.qr>
Date:	Sat, 18 Nov 2006 18:25:25 +0100 (MET)
From:	Jan Engelhardt <jengelh@...ux01.gwdg.de>
To:	"Divy Le Ray <divy@...lsio.com>" <divy@...lsio.com>
cc:	jeff@...zik.org, netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/10] cxgb3 - main header files


On Nov 17 2006 12:23, Divy Le Ray <divy@...lsio.com> wrote:
>Subject: [PATCH 1/10] cxgb3 - main header files

(For all files)

Some suggestions:

 *  change the typedefs to struct, this includes:
        adapter_t -> struct adapter



 *  function prototypes and function headers (e.g. t3_get_cong_cntl_tab)
    are listed as

        void t3_get_cong_cntl_tab(adapter_t *adap,
                         unsigned short incr[NMTUS][NCCTRL_WIN]);

    which could be shortened to

        void t3_get_cong_cntl_tab(adapter_t *adap,
                         unsigned short incr[][]);

    functions where there is only one level of [], e.g.

        void t3_load_mtus(adapter_t *adap, unsigned short mtus[NMTUS],
                  unsigned short alpha[NCCTRL_WIN],
                  unsigned short beta[NCCTRL_WIN], unsigned short mtu_cap);

    could become

        void t3_load_mtus(adapter_t *adap, unsigned short *mtus,
                  unsigned short *alpha,
                  unsigned short *beta, unsigned short mtu_cap);

    depending on your taste.



 *  get rid of superfluous from-void/to-void casts, such as:

        cxgb3_main.c:754:  struct adapter *adapter =
                           (struct adapter *)dev->priv;

    in some places, the 'U' suffix for literal numbers is not required,
    such as:

        cxgb3_main.c:292:   unsigned int nq1 = max((unsigned
                            int)adap->port[1].nqsets, 1U);

        sge.c:2359:  qs->rspq.holdoff_tmr = max(p->coalesce_usecs * 10, 1U);



 *  const run 1: const'ify structs that do not change or
    are not changed, such as

        cxgb3_main.c:924:static char stats_strings[][ETH_GSTRING_LEN] = {

        t3_hw.c:220:static struct mdio_ops mi1_mdio_ops = {
        t3_hw.c:271:static struct mdio_ops mi1_mdio_ext_ops = {

        t3_hw.c:1130:   static struct intr_info pcix1_intr_info[] = {
        t3_hw.c:1166:   static struct intr_info pcie_intr_info[] = {

    (these are just the small picture)



 *  const run 2: const'ify locals that do not change, such as

        cxgb3_offload.c:63:     struct adapter *adapter = tdev2adap(tdev);



 *  const run 3: const'ify function arguments that do not change, e.g.

       static inline int offload_activated(struct t3cdev *tdev)


Note that the listed lines do not cover all source lines - cxgb3
is quite some code.

Running it through sparse gives 'context imbalance' (whatever that is - i just
tried sparse).


	-`J'
-- 
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/