[U-Boot] [PATCH v3 05/21] efi_loader: add device-path utils

Rob Clark
2017-09-20T14:15:02Z

On Wed, Sep 20, 2017 at 4:31 AM, Alexander Graf <agraf@suse.de> wrote:
>
>
> On 14.09.17 00:05, Rob Clark wrote:
>>
>> Helpers to construct device-paths from devices, partitions, files, and
>> for parsing and manipulating device-paths.
>>
>> For non-legacy devices, this will use u-boot's device-model to construct
>> device-paths which include bus hierarchy to construct device-paths. For
>> legacy devices we still fake it, but slightly more convincingly.
>>
>> Signed-off-by: Rob Clark <robdclark@gmail.com>
>
>
> This patch gives me checkpatch warnings left and right (unsigned vs unsigned
> int, double blank lines, double assignments, unsafe define, ...). I'll pull
> it in for now since it seems to be functionally correct, but please fix up
> the warnings in a follow-up patch.
>
>
> Alex
>
> WARNING: Adding new packed members is to be done with care
> #56: FILE: include/efi_api.h:340:
> +} __packed;

fwiw, the __packed warnings are bogus and should be ignored. Maybe
someone can fix checkpatch to whitelist efi_api.h (or just drop that
warning again).

BR,
-R

>
> CHECK: Please don't use multiple blank lines
> #69: FILE: include/efi_loader.h:200:
>
> +
>
> WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
> #74: FILE: include/efi_loader.h:205:
> +unsigned efi_dp_size(const struct efi_device_path *dp);
>
> CHECK: Please don't use multiple blank lines
> #81: FILE: include/efi_loader.h:212:
> +
> +
>
> CHECK: Macro argument reuse '_dp' - possible side-effects?
> #91: FILE: include/efi_loader.h:222:
> +#define EFI_DP_TYPE(_dp, _type, _subtype) \
> + (((_dp)->type == DEVICE_PATH_TYPE_##_type) && \
> + ((_dp)->sub_type == DEVICE_PATH_SUB_TYPE_##_subtype))
>
> WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
> #136:
> new file mode 100644
>
> CHECK: Comparison to NULL could be written "!dp"
> #195: FILE: lib/efi_loader/efi_device_path.c:55:
> + if (dp == NULL)
>
> CHECK: Please don't use multiple blank lines
> #232: FILE: lib/efi_loader/efi_device_path.c:92:
> +
> +
>
> CHECK: Please don't use multiple blank lines
> #301: FILE: lib/efi_loader/efi_device_path.c:161:
> +
> +
>
> WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
> #320: FILE: lib/efi_loader/efi_device_path.c:180:
> +unsigned efi_dp_size(const struct efi_device_path *dp)
>
> WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
> #322: FILE: lib/efi_loader/efi_device_path.c:182:
> + unsigned sz = 0;
>
> WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
> #335: FILE: lib/efi_loader/efi_device_path.c:195:
> + unsigned sz = efi_dp_size(dp) + sizeof(END);
>
> WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
> #357: FILE: lib/efi_loader/efi_device_path.c:217:
> + unsigned sz1 = efi_dp_size(dp1);
>
> WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
> #358: FILE: lib/efi_loader/efi_device_path.c:218:
> + unsigned sz2 = efi_dp_size(dp2);
>
> WARNING: Missing a blank line after declarations
> #360: FILE: lib/efi_loader/efi_device_path.c:220:
> + void *p = dp_alloc(sz1 + sz2 + sizeof(END));
> + memcpy(p, dp1, sz1);
>
> WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
> #379: FILE: lib/efi_loader/efi_device_path.c:239:
> + unsigned sz = node->length;
>
> WARNING: Missing a blank line after declarations
> #381: FILE: lib/efi_loader/efi_device_path.c:241:
> + void *p = dp_alloc(sz + sizeof(END));
> + memcpy(p, node, sz);
>
> WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
> #386: FILE: lib/efi_loader/efi_device_path.c:246:
> + unsigned sz = efi_dp_size(dp);
>
> WARNING: Missing a blank line after declarations
> #388: FILE: lib/efi_loader/efi_device_path.c:248:
> + void *p = dp_alloc(sz + node->length + sizeof(END));
> + memcpy(p, dp, sz);
>
> WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
> #401: FILE: lib/efi_loader/efi_device_path.c:261:
> +static unsigned dp_size(struct udevice *dev)
>
> CHECK: multiple assignments should be avoided
> #484: FILE: lib/efi_loader/efi_device_path.c:344:
> + start = buf = dp_alloc(dp_size(dev) + sizeof(END));
>
> WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
> #492: FILE: lib/efi_loader/efi_device_path.c:352:
> +static unsigned dp_part_size(struct blk_desc *desc, int part)
>
> WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
> #494: FILE: lib/efi_loader/efi_device_path.c:354:
> + unsigned dpsize;
>
> CHECK: Please don't use multiple blank lines
> #581: FILE: lib/efi_loader/efi_device_path.c:441:
> +
> +
>
> CHECK: multiple assignments should be avoided
> #587: FILE: lib/efi_loader/efi_device_path.c:447:
> + start = buf = dp_alloc(dp_part_size(desc, part) + sizeof(END));
>
> WARNING: Missing a blank line after declarations
> #601: FILE: lib/efi_loader/efi_device_path.c:461:
> + char c = *(path++);
> + if (c == '/')
>
> CHECK: Alignment should match open parenthesis
> #613: FILE: lib/efi_loader/efi_device_path.c:473:
> +struct efi_device_path *efi_dp_from_file(struct blk_desc *desc, int part,
> + const char *path)
>
> WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
> #617: FILE: lib/efi_loader/efi_device_path.c:477:
> + unsigned dpsize = 0, fpsize;
>
> CHECK: multiple assignments should be avoided
> #625: FILE: lib/efi_loader/efi_device_path.c:485:
> + start = buf = dp_alloc(dpsize + sizeof(END));
>
> WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
> #648: FILE: lib/efi_loader/efi_device_path.c:508:
> + unsigned dpsize = 0;
>
> CHECK: multiple assignments should be avoided
> #659: FILE: lib/efi_loader/efi_device_path.c:519:
> + start = buf = dp_alloc(dpsize + sizeof(END));
>
> total: 0 errors, 19 warnings, 12 checks, 644 lines checked
>
> NOTE: For some of the reported defects, checkpatch may be able to
> mechanically convert to the typical style using --fix or
> --fix-inplace.
>
> Your patch has style problems, please review.
>
> NOTE: Ignored message types: COMPLEX_MACRO CONSIDER_KSTRTO MINMAX
> MULTISTATEMENT_MACRO_USE_DO_WHILE NETWORKING_BLOCK_COMMENT_STYLE
> PREFER_ETHER_ADDR_COPY USLEEP_RANGE
>
> NOTE: If any of the errors are false positives, please report
> them to the maintainer, see CHECKPATCH in MAINTAINERS.
>