Skip to content

Commit

Permalink
lib/alg-yescrypt-platform.c: Fix -Werror=sign-conversion.
Browse files Browse the repository at this point in the history 
In 894aee7 we introduced some
changes, which show is this error when building with GCC v12.2.1.
  • Loading branch information
@besser82
besser82 committed Nov 18, 2022
1 parent ebe12d8 commit 05e5705
Showing 1 changed file with 2 additions and 2 deletions.
4 changes: 2 additions & 2 deletions lib/alg-yescrypt-platform.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ static void *alloc_region(yescrypt_region_t *region, size_t size)
size_t new_size = size;
const size_t hugepage_mask = (size_t)HUGEPAGE_SIZE - 1;
if (size >= HUGEPAGE_THRESHOLD && size + hugepage_mask >= size) {
flags |= MAP_HUGETLB | MAP_HUGE_2MB;
flags |= (int)(MAP_HUGETLB | MAP_HUGE_2MB);
/*
* Linux's munmap() fails on MAP_HUGETLB mappings if size is not a multiple of
* huge page size, so let's round up to huge page size here.
Expand All @@ -59,7 +59,7 @@ static void *alloc_region(yescrypt_region_t *region, size_t size)
if (base != MAP_FAILED) {
base_size = new_size;
} else if (flags & MAP_HUGETLB) {
flags &= ~(MAP_HUGETLB | MAP_HUGE_2MB);
flags &= ~(int)(MAP_HUGETLB | MAP_HUGE_2MB);
base = mmap(NULL, size, PROT_READ | PROT_WRITE, flags, -1, 0);
}

Expand Down

3 comments on commit 05e5705

@besser82
Copy link
Owner Author

Choose a reason for hiding this comment

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

@solardiz FYI.

@solardiz
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks @besser82. This is nasty stuff (mmap is specified to accept an int for flags, but applying ~ to a signed type is... not ideal, and can lead to other warnings), and one more example that makes me question the usefulness of that warning. I actually did test the MAP_HUGE_2MB fix on a gcc 12 system, but didn't have that warning enabled - if I enable it, there are many more to fix. I don't know if all of those are already fixed in libxcrypt - I know one is (#161), which I was about to fix as well but didn't yet because there are so many more anyway.

I guess instead of your:

		flags &= ~(int)(MAP_HUGETLB | MAP_HUGE_2MB);

I'd try something like:

		flags &= (int)~(MAP_HUGETLB | MAP_HUGE_2MB);

or even:

		flags &= (int)~(unsigned int)(MAP_HUGETLB | MAP_HUGE_2MB);

Doesn't look pretty. :-(

or maybe use unsigned int flags and then cast to (int) on the call to mmap.

@besser82
Copy link
Owner Author

Choose a reason for hiding this comment

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

or maybe use unsigned int flags and then cast to (int) on the call to mmap.

You're welcome! I've changed it as recommended in 53cff4b.

Please sign in to comment.