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

[NOSQUASH] Fix some warnings #13586

Merged
merged 3 commits into from Jun 15, 2023
Merged

[NOSQUASH] Fix some warnings #13586

merged 3 commits into from Jun 15, 2023

Conversation

Desour
Copy link
Member

@Desour Desour commented Jun 12, 2023

  • An intentional self-move in the irr_ptr unittests.
    (Note: If not defined, macros, such as __GNUC__, are replaced 0, so this is fine.)
  • The minimum curl version is now specified as 7.68.0. This is the version on ubuntu 20.04.
    The actual minimum is probably 7.56.0, but I'm not sure if the other parts of the code use newer functions.
    FYI, ubuntu 18.04 seems to have curl 7.58.0.

    The minimum curl version is now specified as 7.56.0.
  • The invlist warnings were introduced recently.
    (I'm assuming invlist size does not use the full range of values in u32. We should really start using one datatype for invlist indices everywhere at some point...)

To do

This PR is a Ready for Review.

How to test

  • Build.
  • Example to try the multipart http stuff:
local http = assert(minetest.request_http_api())

minetest.after(3, function()
	local req = {
		url = "https://httpbin.org/post",
		method = "POST",
		data = {foo = "bar", baz = "buzz", bla = "bli"},
		user_agent = " ",
		multipart = true,
	}
	http.fetch(req, function(res)
		minetest.log(dump(res))
	end)
	minetest.log("sent http request")
end)

@Desour Desour added the Maintenance Tasks to keep the codebase and related parts in order, including architectural improvements label Jun 12, 2023
@JosiahWI
Copy link
Contributor

I had noticed the warnings and they were distracting me. I like the prospect of having these fixed.

@@ -4376,7 +4376,7 @@ bool GUIFormSpecMenu::OnEvent(const SEvent& event)

ItemStack slct = list_selected->getItem(m_selected_item->i);

for (s32 i = 0; i < list_s->getSize(); i++) {
for (s32 i = 0; i < (s32)list_s->getSize(); i++) {
Copy link
Contributor

Choose a reason for hiding this comment

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

What type does getSize return? We do not need 32 bits here, should change the type of i instead?

Copy link
Member Author

Choose a reason for hiding this comment

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

getSize returns an u32.
InventoryList (see inventory.h) seems to use u32 everywhere. But there's also for example MoveAction (see inventrymanager.h), which uses s16.
It would probably be good to use s32 everywhere (after all u16 could get too small for things like creative inventories). But doing that change would require more work.

doc/compiling/linux.md Outdated Show resolved Hide resolved
Copy link
Member

@sfan5 sfan5 left a comment

Choose a reason for hiding this comment

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

LGTM

@Desour Desour merged commit c549e84 into minetest:master Jun 15, 2023
13 checks passed
@Desour Desour deleted the fix_warnings2 branch June 15, 2023 11:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Maintenance Tasks to keep the codebase and related parts in order, including architectural improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants