https://bugzilla.redhat.com/show_bug.cgi?id=1835452
Spencer Lingard <spencer(a)mellanox.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Flags|needinfo?(spencer@mellanox. |
|com) |
--- Comment #6 from Spencer Lingard <spencer(a)mellanox.com> ---
Spec URL:
https://download.copr.fedorainfracloud.org/results/slingard/mlxbf-bootctl...
SRPM URL:
https://download.copr.fedorainfracloud.org/results/slingard/mlxbf-bootctl...
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....).
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(a)mellanox.com> 1.1-4
41 (none)
42
43 * Tue May 12 2020 Spencer Lingard <spencer(a)mellanox.com> 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