Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix some minor compiler warnings #548

Merged
merged 4 commits into from
Nov 14, 2024

Conversation

ddiss
Copy link

@ddiss ddiss commented Nov 14, 2024

The following changes since commit 699f87c5ee5f2302e615f5e7dae671fc959adf80:

  Merge pull request #545 from ddiss/lklfuse_vfs_open_flags (2024-06-27 00:14:48 -0700)

are available in the Git repository at:

  https://github.com/ddiss/linux fix_some_minor_compiler_warnings

for you to fetch changes up to b1339686dbc0866caf49c0fd57b70fbceb543680:

  lkl: silence bootmem compiler warning (2024-11-14 05:36:44 +0100)

----------------------------------------------------------------
David Disseldorp (4):
      lkl: fix vfio_pci warnings
      lkl: minor vfio_pci error handling improvements
      lkl: silence compiler warning
      lkl: silence bootmem compiler warning

 arch/lkl/kernel/misc.c   |  1 -
 arch/lkl/mm/bootmem.c    | 19 ++++++++++---------
 tools/lkl/lib/vfio_pci.c | 12 ++++++++----
 3 files changed, 18 insertions(+), 14 deletions(-)

Calling snprintf() with overlapping source and destination buffers is
undefined behavior, as per C/POSIX specs. Avoid this by adding a second
buffer, and also eliminate a compiler warning from similar readlink()
source-is-dest logic:

lib/vfio_pci.c: In function ‘vfio_pci_add’:
lib/vfio_pci.c:80:28: warning: passing argument 2 to
‘restrict’-qualified parameter aliases with argument 1 [-Wrestrict]
   80 |         i = readlink(path, path, sizeof(path));
      |                      ~~~~  ^~~~

Signed-off-by: David Disseldorp <[email protected]>
readlink() can fill up to sizeof(link), in which case the nulterm would
overflow. Fix this and avoid assuming that the link carries a '/'.

Signed-off-by: David Disseldorp <[email protected]>
arch/lkl/kernel/misc.c: In function ‘wrong_size_cmpxchg’:
arch/lkl/kernel/misc.c:18:16: warning: function declared ‘noreturn’ has
a ‘return’ statement
   18 |         return 0;

Drop the return after panic().

Signed-off-by: David Disseldorp <[email protected]>
gcc-14 complains about the type mismatch:
arch/lkl/mm/bootmem.c: In function ‘bootmem_init’:
arch/lkl/mm/bootmem.c:39:35: error: passing argument 1 of ‘virt_to_pfn’
makes pointer from integer without a cast [-Wint-conversion]
   39 |         max_low_pfn = virt_to_pfn(memory_end);
      |                                   ^~~~~~~~~~
      |                                   |
      |                                   long unsigned int
...
./include/asm-generic/page.h:77:53: note: expected ‘const void *’ but
argument is of type ‘long unsigned int’
   77 | static inline unsigned long virt_to_pfn(const void *kaddr)
      |                                         ~~~~~~~~~~~~^~~~~

Add a cast to silence the warning. Drop some unnecessary casts for
_memory_start and empty_zero_page, which can both be void *.

Signed-off-by: David Disseldorp <[email protected]>
@ddiss
Copy link
Author

ddiss commented Nov 14, 2024

I should mention the vfio_pci changes are compile tested only.

Copy link
Member

@tavip tavip left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks David!

@tavip
Copy link
Member

tavip commented Nov 14, 2024

Github tests are failing because 'actions/upload-artifact: v2' has been deprecated. ci/circleci/x86_64_qemu has known flakiness issues. So all looks good and we can merge.

@tavip tavip merged commit 63bed8b into lkl:master Nov 14, 2024
6 of 13 checks passed
@ddiss
Copy link
Author

ddiss commented Nov 19, 2024

Github tests are failing because 'actions/upload-artifact: v2' has been deprecated. ci/circleci/x86_64_qemu has known flakiness issues. So all looks good and we can merge.

Thanks for the review and merge, Octavian! I've raised #550 to track the Github test failures.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants