Vasu Dev <[email protected]> writes:
> On Thu, 2015-10-01 at 15:17 +0200, Johannes Thumshirn wrote:
>> This patch series replaces all usage of libHBAAPIv2 and libhbalinux2 from
>> fcoe-utils and replaces them with an internal version operating directly on
>> the
>> respective sysfs files.
>>
>> This removes the two dependencies but pulls in libpciacces (which got pulled
>> in
>> by libhbalinux2 in the current version of fcoe-utils). Nevertheless this way
>> it
>> is possible to get rid of a lot of code.
>>
>> Changes to v2:
>> o Do not hardcode /sys/class/net/<device>/ctlr_<num>/ to ctlr_0 but read out
>> of
>> sysfs
>
> Just in time since now I've get go to proceed with removing HBAAPI in
> regard to related any certification impact. Your patch series applies
> cleanly but very first patch has lots of style issues. I guess many with
> strings/printf over lines 80 col and those can be ignored but if
> possible then fix them as well beside all other checkpatch warnings:-
>
> Also, along checkpatch style fixes have defines moved to header files
> and avoid adding them in the C files.
Oh fuuuu. I completely forgot on running checkpatch on it, you're right.
I'll post a v4 later today including style fixes, move of defines and
squash of the last 2 patches into 1 (no idea why there are 2 patches
with the same commit message, it should've landed in one).
>
> For instances very first patch has these checkpatch issues:-
>
> WARNING: line over 80 characters
> #224: FILE: lib/sysfs_hba.c:68:
> + fprintf(stderr, "Failed reading PCI Capability List
> Register\n");
>
> WARNING: line over 80 characters
> #234: FILE: lib/sysfs_hba.c:78:
> + rc = pci_device_cfg_read_u8(dev, &cap_id, offset +
> PCI_CAP_LIST_ID);
>
> WARNING: line over 80 characters
> #236: FILE: lib/sysfs_hba.c:80:
> + fprintf(stderr, "Failed reading capability ID at
> 0x%"PRIx64"\n",
>
> WARNING: line over 80 characters
> #245: FILE: lib/sysfs_hba.c:89:
> + fprintf(stderr, "Failed reading next
> capability ID at 0x%"PRIx64"\n",
>
> WARNING: line over 80 characters
> #256: FILE: lib/sysfs_hba.c:100:
> + rc = pci_device_cfg_read_u32(dev,
> &pcie_cap_header, offset);
>
> WARNING: line over 80 characters
> #258: FILE: lib/sysfs_hba.c:102:
> + fprintf(stderr, "Failed reading PCIe
> config header\n");
>
> WARNING: line over 80 characters
> #269: FILE: lib/sysfs_hba.c:113:
> + (void) pci_device_cfg_read_u32(dev, &dword_low,
> offset + 4);
>
> WARNING: line over 80 characters
> #270: FILE: lib/sysfs_hba.c:114:
> + (void) pci_device_cfg_read_u32(dev, &dword_high,
> offset + 8);
> WARNING: line over 80 characters
> #271: FILE: lib/sysfs_hba.c:115:
> + snprintf(info->serial_number,
> sizeof(info->serial_number),
>
> WARNING: line over 80 characters
> #274: FILE: lib/sysfs_hba.c:118:
> + (dword_high >> 8) & 0xff, (dword_low >>
> 16) & 0xff,
>
> WARNING: void function return statements are not generally useful
> #341: FILE: lib/sysfs_hba.c:185:
> + return;
> +}
>
> WARNING: line over 80 characters
> #359: FILE: lib/sysfs_hba.c:203:
> + sscanf(pcidev, "%x:%x:%x.%x", &match.domain, &match.bus,
> &match.dev, &match.func);
>
> WARNING: unchecked sscanf return value
> #359: FILE: lib/sysfs_hba.c:203:
> + sscanf(pcidev, "%x:%x:%x.%x", &match.domain, &match.bus,
> &match.dev, &match.func);
>
> WARNING: line over 80 characters
> #397: FILE: lib/sysfs_hba.c:241:
> + sa_sys_read_line(path, "symbolic_name", pa->symbolic_name,
> sizeof(pa->symbolic_name));
>
> WARNING: line over 80 characters
> #398: FILE: lib/sysfs_hba.c:242:
> + sa_sys_read_line(path, "node_name", pa->node_name,
> sizeof(pa->node_name));
>
> WARNING: line over 80 characters
> #399: FILE: lib/sysfs_hba.c:243:
> + sa_sys_read_line(path, "port_name", pa->port_name,
> sizeof(pa->port_name));
>
> WARNING: line over 80 characters
> #400: FILE: lib/sysfs_hba.c:244:
> + sa_sys_read_line(path, "fabric_name", pa->fabric_name,
> sizeof(pa->fabric_name));
>
> WARNING: line over 80 characters
> #402: FILE: lib/sysfs_hba.c:246:
> + sa_sys_read_line(path, "supported_speeds", pa->supported_speeds,
> sizeof(pa->supported_speeds));
>
> WARNING: line over 80 characters
> #403: FILE: lib/sysfs_hba.c:247:
> + sa_sys_read_line(path, "maxframe_size", pa->maxframe_size,
> sizeof(pa->maxframe_size));
>
> WARNING: line over 80 characters
> #405: FILE: lib/sysfs_hba.c:249:
> + sa_sys_read_line(path, "port_state", pa->port_state,
> sizeof(pa->port_state));
>
> ERROR: need consistent spacing around '-' (ctx:WxV)
> #447: FILE: lib/sysfs_hba.c:291:
> + ret = readlink(path, buf, sizeof(buf) -1 );
> ^
>
> ERROR: space prohibited before that close parenthesis ')'
> #447: FILE: lib/sysfs_hba.c:291:
> + ret = readlink(path, buf, sizeof(buf) -1 );
>
> ERROR: space required before the open parenthesis '('
> #464: FILE: lib/sysfs_hba.c:308:
> + } while(cp && cp > buf);
>
> ERROR: space required before the open parenthesis '('
> #482: FILE: lib/sysfs_hba.c:326:
> + if(ret == -1)
>
> ERROR: trailing statements should be on next line
> #493: FILE: lib/sysfs_hba.c:337:
> + if (dp->d_name[0] == '.' && dp->d_name[1] == '\0')
> continue;
>
> ERROR: trailing statements should be on next line
> #494: FILE: lib/sysfs_hba.c:338:
> + if (dp->d_name[1] == '.' && dp->d_name[2] == '\0')
> continue;
>
> ERROR: trailing statements should be on next line
> #533: FILE: lib/sysfs_hba.c:377:
> + if (dp->d_name[0] == '.' && dp->d_name[1] == '\0')
> continue;
>
> ERROR: trailing statements should be on next line
> #534: FILE: lib/sysfs_hba.c:378:
> + if (dp->d_name[1] == '.' && dp->d_name[2] == '\0')
> continue;
>
> total: 12 errors, 26 warnings, 494 lines checked
>
> /home/vasu/src/fcoe-utils/patches-master/01-fcoe-utils-add-sysfs_hba-to-to
> has style problems, please review.
>
>
>
--
Johannes Thumshirn Storage
[email protected] +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
_______________________________________________
fcoe-devel mailing list
[email protected]
http://lists.open-fcoe.org/mailman/listinfo/fcoe-devel
