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

libc cleanup #1568

Merged
merged 3 commits into from
Oct 27, 2023
Merged

libc cleanup #1568

merged 3 commits into from
Oct 27, 2023

Conversation

Thar0
Copy link
Contributor

@Thar0 Thar0 commented Oct 23, 2023

Mostly contains the libc changes from #1468, to try and cut that one down to be more manageable.

include/functions.h Show resolved Hide resolved
include/libc/assert.h Show resolved Hide resolved
include/libc/assert.h Show resolved Hide resolved
Comment on lines 7 to 9
#define M_SQRT2 1.41421356237309504880f
#define FLT_MAX 340282346638528859811704183484516925440.0f
#define MAXFLOAT 3.40282347e+38f
#define SHT_MAX 32767.0f
Copy link
Collaborator

Choose a reason for hiding this comment

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

is this a known original name or a standard name for this?
am fine with it if so, but it just goes against our convention for similar things

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the standard C name for the largest representable float define in math.h.
FLT_MAX is another standard C name, but is part of float.h instead, which we don't currently have. It might be nice to add additional standard headers in the future, but there's some questions about exactly how much to add so I opted not to add too many new headers for now.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Specifically ISO C has FLT_MAX in float.h, which is not a header we currently have. I think it may be nice to add float.h and limits.h at some point, or I can attempt to add them now if it's much desired?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm fine with the way it is now personally

Copy link
Collaborator

Choose a reason for hiding this comment

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

What is the meaning of this file name?

I see you put bzero/bcmp/bcopy in here which apparently were posix and today are deprecated/removed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

idk, it's a libultra header so I could only speculate

src/libultra/libc/xprintf.c Outdated Show resolved Hide resolved

if (*(u16*)&px->v.ll & 0x8000) {
if (LDSIGN(px->v.ld)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

why make a macro for this? and is there a reason to not do ld < 0 (besides, for us, matching)

this with the macro isdigit (not IS_DIGIT) makes me wonder if you yoinked this code from somewhere or are looking at a reference?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess they wanted to avoid float operations, possibly for NaN/infinity handling. I tried to sync a bit more with ultralib's xprintf hence the macros

src/libultra/libc/xlitob.c Outdated Show resolved Hide resolved
src/libultra/libc/sprintf.c Outdated Show resolved Hide resolved
return l / r;
}

s64 __ll_lshift(s64 l, s64 r) {
unsigned long long __ll_lshift(unsigned long long l, unsigned long long r) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

you changed the types from signed to unsigned here and in other functions below, was that intended?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wanted to better match ultralib, but looking at the libultra debug info the current types in ultralib seem a bit wrong

include/ultra64/xstdio.h Outdated Show resolved Hide resolved
@@ -46,7 +46,7 @@ s32 Overlay_Load(uintptr_t vromStart, uintptr_t vromEnd, void* vramStart, void*
// "Clear BSS area (% 08x-% 08x)"
osSyncPrintf("BSS領域をクリアします(%08x-%08x)\n", end, end + ovlRelocs->bssSize);
}
bzero((void*)end, ovlRelocs->bssSize);
bzero((void*)end, (s32)ovlRelocs->bssSize);
Copy link
Collaborator

Choose a reason for hiding this comment

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

bruh

Copy link
Collaborator

@Dragorn421 Dragorn421 left a comment

Choose a reason for hiding this comment

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

Need to yeet the printf.h include directive

@Dragorn421 Dragorn421 merged commit 3475651 into zeldaret:main Oct 27, 2023
1 check passed
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.

4 participants