https://bugzilla.redhat.com/show_bug.cgi?id=1835452 Spencer Lingard <spencer@xxxxxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(spencer@mellanox. | |com) | --- Comment #6 from Spencer Lingard <spencer@xxxxxxxxxxxx> --- Spec URL: https://download.copr.fedorainfracloud.org/results/slingard/mlxbf-bootctl/fedora-rawhide-aarch64/01476093-mlxbf-bootctl/mlxbf-bootctl.spec SRPM URL: https://download.copr.fedorainfracloud.org/results/slingard/mlxbf-bootctl/fedora-rawhide-aarch64/01476093-mlxbf-bootctl/mlxbf-bootctl-1.1-5.fc33.src.rpm Scratch Koji URL: https://koji.fedoraproject.org/koji/taskinfo?taskID=45835273 New version for review. (In reply to Honggang LI from comment #2) > 1 Name: mlxbf-bootctl > 2 Version: 1.1 > 3 %{!?_release: %define _release 4} > 4 Release: %{_release}%{?dist} > Please delete line 3, and replace "%{_release}" with 4 for line 4. Done. > > 5 Summary: Mellanox BlueField boot partition management utility > 6 > 7 License: BSD > 8 Url: https://github.com/Mellanox/mlxbf-bootctl > 9 Source: mlxbf-bootctl-1.1.tar.gz > 10 > 11 ExclusiveArch: aarch64 > > Need a comment for "ExclusiveArch", see > https://fedoraproject.org/wiki/Packaging:ReviewGuidelines Done. > > 12 > 13 BuildRequires: binutils > 14 BuildRequires: gcc > > line 13 should be deleted, as gcc requires binutils. > > $ rpm -qR gcc | grep binutils > binutils >= 2.31 Done. > > 15 > 16 %description > 17 Access to all the boot partition management is via a program shipped > 18 with the BlueField software called "mlxbf-bootctl". > > I have no idea what is main function or feature of this package after read > this > '%description' section. Please improve it. Done. > > 19 > 20 %prep > 21 %setup -q -n mlxbf-bootctl-1.1 > > "%setup -q" should be enough, in case > 1) top directory name was in format "%{name}-%{version}/" > 2) tarball name was in format "%{name}-%{version}.XXX" This spec file is generated by rpkg (https://github.com/Mellanox/mlxbf-bootctl/blob/master/mlxbf-bootctl.spec.rpkg). As far as I can tell, I can't change the way rpkg generates the %setup macro. > > 22 > 23 %build > 24 %make_build > 25 > 26 %install > 27 %make_install > 28 %{__install} -d %{buildroot}%{_mandir}/man8 > 29 %{__install} -m 0644 mlxbf-bootctl.8 %{buildroot}%{_mandir}/man8 > 30 > 31 %files > 32 %defattr(-, root, root) > line 32 is unnecessary, please remove it. Done. > > 33 /sbin/* > should install programs in %{_sbindir}, and use %{_sbindir}/XXX, XXX is the > program name. Done. > > 34 %{_mandir}/man8/mlxbf-bootctl.8.gz > 35 > 36 %license LICENSE > 37 %doc mlxbf-bootctl.txt > 38 > 39 %changelog > 40 * Wed Jun 10 2020 Spencer Lingard <spencer@xxxxxxxxxxxx> 1.1-4 > 41 (none) > 42 > 43 * Tue May 12 2020 Spencer Lingard <spencer@xxxxxxxxxxxx> 1.1-3 > 44 (none) > > line 41 and 44 are unnecessary, should be deleted. Changed them to be more detailed. (In reply to Honggang LI from comment #3) > Task URL: https://cov01.lab.eng.brq.redhat.com/covscanhub/task/175416/ > Comment: None > > > All defects > > CHECKED_RETURN 1 > CLANG_WARNING 1 > CPPCHECK_WARNING 1 > > ========================================================================== > mlxbf-bootctl-1.1-4.fc31 > List of Defects > > Error: CPPCHECK_WARNING (CWE-664): [#def1] > mlxbf-bootctl-1.1/mlxbf-bootctl.c:60: error[va_end_missing]: va_list 'ap' > was opened but not closed by va_end(). > # 58| putc('\n', stderr); > # 59| exit(1); > # 60|-> } > # 61| > # 62| #ifndef OUTPUT_ONLY Fixed. > > Error: CLANG_WARNING: [#def2] > mlxbf-bootctl-1.1/mlxbf-bootctl.c:545:5: warning: Null pointer passed to 1st > parameter expecting 'nonnull' > # memset(buf + seg_size, 0, pad_size); > # ^ This is a false positive. The tool assumes that (buf == NULL) at line 524, but if buf == NULL, the program will die with an error message rather than continuing. See the note below. > mlxbf-bootctl-1.1/mlxbf-bootctl.c:644:10: note: Assuming the condition is > true > # while ((opt = getopt_long(argc, argv, short_options, long_options, NULL)) > # ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > mlxbf-bootctl-1.1/mlxbf-bootctl.c:644:3: note: Loop condition is true. > Entering loop body > # while ((opt = getopt_long(argc, argv, short_options, long_options, NULL)) > # ^ > mlxbf-bootctl-1.1/mlxbf-bootctl.c:647:5: note: Control jumps to 'case 98:' > at line 663 > # switch (opt) > # ^ > mlxbf-bootctl-1.1/mlxbf-bootctl.c:665:7: note: Execution continues on line > 644 > # break; > # ^ > mlxbf-bootctl-1.1/mlxbf-bootctl.c:644:10: note: Assuming the condition is > false > # while ((opt = getopt_long(argc, argv, short_options, long_options, NULL)) > # ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > mlxbf-bootctl-1.1/mlxbf-bootctl.c:644:3: note: Loop condition is false. > Execution continues on line 694 > # while ((opt = getopt_long(argc, argv, short_options, long_options, NULL)) > # ^ > mlxbf-bootctl-1.1/mlxbf-bootctl.c:694:7: note: Assuming 'bootstream' is > non-null > # if (!bootstream && !swap && watchdog_swap == NULL && !watchdog_disable) > # ^~~~~~~~~~~ > mlxbf-bootctl-1.1/mlxbf-bootctl.c:694:19: note: Left side of '&&' is false > # if (!bootstream && !swap && watchdog_swap == NULL && !watchdog_disable) > # ^ > mlxbf-bootctl-1.1/mlxbf-bootctl.c:700:7: note: 'bootstream' is non-null > # if (bootstream) > # ^~~~~~~~~~ > mlxbf-bootctl-1.1/mlxbf-bootctl.c:700:3: note: Taking true branch > # if (bootstream) > # ^ > mlxbf-bootctl-1.1/mlxbf-bootctl.c:702:9: note: 'input_file' is null > # if (input_file) > # ^~~~~~~~~~ > mlxbf-bootctl-1.1/mlxbf-bootctl.c:702:5: note: Taking false branch > # if (input_file) > # ^ > mlxbf-bootctl-1.1/mlxbf-bootctl.c:707:14: note: 'output_file' is null > # else if (output_file) > # ^~~~~~~~~~~ > mlxbf-bootctl-1.1/mlxbf-bootctl.c:707:10: note: Taking false branch > # else if (output_file) > # ^ > mlxbf-bootctl-1.1/mlxbf-bootctl.c:718:11: note: Assuming 'boot_part_size' is > >= field 'st_size' > # if (st.st_size > boot_part_size) > # ^~~~~~~~~~~~~~~~~~~~~~~~~~~ > mlxbf-bootctl-1.1/mlxbf-bootctl.c:718:7: note: Taking false branch > # if (st.st_size > boot_part_size) > # ^ > mlxbf-bootctl-1.1/mlxbf-bootctl.c:725:11: note: Assuming the condition is > false > # if (asprintf(&bootfile, "%sboot%d", mmc_path, boot_part ^ which_boot) > <= 0) > # > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > mlxbf-bootctl-1.1/mlxbf-bootctl.c:725:7: note: Taking false branch > # if (asprintf(&bootfile, "%sboot%d", mmc_path, boot_part ^ which_boot) > <= 0) > # ^ > mlxbf-bootctl-1.1/mlxbf-bootctl.c:728:7: note: Calling 'write_bootstream' > # write_bootstream(bootstream, bootfile, O_SYNC); > # ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > mlxbf-bootctl-1.1/mlxbf-bootctl.c:475:3: note: Taking true branch > # if (strncmp(bootfile, "/dev/", 5) == 0) > # ^ > mlxbf-bootctl-1.1/mlxbf-bootctl.c:477:9: note: Assuming the condition is > false > # if (asprintf(&sysname, "/sys/block/%s/force_ro", &bootfile[5]) <= 0) > # ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > mlxbf-bootctl-1.1/mlxbf-bootctl.c:477:5: note: Taking false branch > # if (asprintf(&sysname, "/sys/block/%s/force_ro", &bootfile[5]) <= 0) > # ^ > mlxbf-bootctl-1.1/mlxbf-bootctl.c:481:9: note: Assuming 'sysfd' is < 0 > # if (sysfd >= 0) > # ^~~~~~~~~~ > mlxbf-bootctl-1.1/mlxbf-bootctl.c:481:5: note: Taking false branch > # if (sysfd >= 0) > # ^ > mlxbf-bootctl-1.1/mlxbf-bootctl.c:503:11: note: Assuming the condition is > false > # if (errno != ENOENT) > # ^~~~~~~~~~~~~~~ > /usr/include/errno.h:38:16: note: expanded from macro 'errno' > ## define errno (*__errno_location ()) > # ^ > mlxbf-bootctl-1.1/mlxbf-bootctl.c:503:7: note: Taking false branch > # if (errno != ENOENT) > # ^ > mlxbf-bootctl-1.1/mlxbf-bootctl.c:513:7: note: Assuming 'ifd' is < 0 > # if (ifd < 0) > # ^~~~~~~ > mlxbf-bootctl-1.1/mlxbf-bootctl.c:513:3: note: Taking true branch > # if (ifd < 0) > # ^ > mlxbf-bootctl-1.1/mlxbf-bootctl.c:516:7: note: Assuming 'ofd' is >= 0 > # if (ofd < 0) > # ^~~~~~~ > mlxbf-bootctl-1.1/mlxbf-bootctl.c:516:3: note: Taking false branch > # if (ofd < 0) > # ^ > mlxbf-bootctl-1.1/mlxbf-bootctl.c:519:7: note: Assuming the condition is > false > # if (fstat(ifd, &st) < 0) > # ^~~~~~~~~~~~~~~~~~~ > mlxbf-bootctl-1.1/mlxbf-bootctl.c:519:3: note: Taking false branch > # if (fstat(ifd, &st) < 0) > # ^ > mlxbf-bootctl-1.1/mlxbf-bootctl.c:523:3: note: 'buf' initialized here > # char *buf = malloc(MAX_SEG_LEN); > # ^~~~~~~~~ > mlxbf-bootctl-1.1/mlxbf-bootctl.c:524:7: note: Assuming 'buf' is equal to > NULL > # if (buf == NULL) > # ^~~~~~~~~~~ Note: if buf == NULL here, the true branch will stop the program, and a NULL pointer will not be passed to memset. > mlxbf-bootctl-1.1/mlxbf-bootctl.c:524:3: note: Taking true branch > # if (buf == NULL) > # ^ > mlxbf-bootctl-1.1/mlxbf-bootctl.c:532:10: note: Assuming 'bytes_left' is > 0 > # while (bytes_left > 0) > # ^~~~~~~~~~~~~~ > mlxbf-bootctl-1.1/mlxbf-bootctl.c:532:3: note: Loop condition is true. > Entering loop body > # while (bytes_left > 0) > # ^ > mlxbf-bootctl-1.1/mlxbf-bootctl.c:534:24: note: Assuming the condition is > false > # size_t seg_size = (bytes_left <= MAX_SEG_LEN) ? bytes_left : > MAX_SEG_LEN; > # ^~~~~~~~~~~~~~~~~~~~~~~~~ > mlxbf-bootctl-1.1/mlxbf-bootctl.c:534:23: note: '?' condition is false > # size_t seg_size = (bytes_left <= MAX_SEG_LEN) ? bytes_left : > MAX_SEG_LEN; > # ^ > mlxbf-bootctl-1.1/mlxbf-bootctl.c:538:23: note: '?' condition is false > # size_t pad_size = seg_size % 8 ? (8 - seg_size % 8) : 0; > # ^ > mlxbf-bootctl-1.1/mlxbf-bootctl.c:539:41: note: 'bytes_left' is not equal to > 0 > # uint64_t segheader = gen_seg_header(bytes_left == 0, 1, BOOT_FIFO_ADDR, > # ^~~~~~~~~~ > mlxbf-bootctl-1.1/mlxbf-bootctl.c:545:5: note: Null pointer passed to 1st > parameter expecting 'nonnull' > # memset(buf + seg_size, 0, pad_size); > # ^ ~~~~~~~~~~~~~~ > # 543| // Copy the segment plus any padding. > # 544| read_or_die(bootstream, ifd, buf, seg_size); > # 545|-> memset(buf + seg_size, 0, pad_size); > # 546| write_or_die(bootfile, ofd, buf, seg_size + pad_size); > # 547| } > > Error: CHECKED_RETURN (CWE-252): [#def3] > mlxbf-bootctl-1.1/mlxbf-bootctl.c:717: check_return: Calling > "stat(bootstream, &st)" without checking return value. This library function > may fail and return an error code. [Note: The source code implementation of > the function has been overridden by a builtin model.] > # 715| uint64_t boot_part_size = get_boot_partition_size(); > # 716| struct stat st; > # 717|-> stat(bootstream, &st); > # 718| if (st.st_size > boot_part_size) > # 719| die("Size of bootstream exceeds boot partition size"); Fixed. -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component _______________________________________________ package-review mailing list -- package-review@xxxxxxxxxxxxxxxxxxxxxxx To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@xxxxxxxxxxxxxxxxxxxxxxx
