Have getters in GX use const pointers

[1011X]

No description provided.

[endrift]

You put the const on the wrong side of the pointer. This just marks the variable itself as const, not the memory it points to, which is useless inside the function.

[mtheall]

I would also suggest adding const to the casts as well, e.g.:

-void* GX_GetFifoBase(GXFifoObj *fifo)
+void* GX_GetFifoBase(const GXFifoObj *fifo)
 {
-	return (void*)((struct __gxfifo*)fifo)->buf_start;
+	return (void*)((const struct __gxfifo*)fifo)->buf_start;
 }

[1011X]

@endrift Gah, thank you. It's been a while since I've written C.

I would also suggest adding const to the casts as well, e.g.:

-void* GX_GetFifoBase(GXFifoObj *fifo)
+void* GX_GetFifoBase(const GXFifoObj *fifo)
 {
-	return (void*)((struct __gxfifo*)fifo)->buf_start;
+	return (void*)((const struct __gxfifo*)fifo)->buf_start;
 }

@mtheall Okay, I'll do that 👍

[endrift]

I think the ones that output void* break const correctness, but still "work" because they're explicitly cast to void* (which you shouldn't actually need to do in C, and would recommend removing those casts in the process to see if it raises warnings).

[mtheall]

I think the ones that output void* break const correctness, but still "work" because they're explicitly cast to void* (which you shouldn't actually need to do in C, and would recommend removing those casts in the process to see if it raises warnings).

Those are fine. fifo is pointer to const but that doesn't make fifo->buf_start pointer to const. The void* cast is needed because the member is actually a vu32 not a pointer

[1011X]

@endrift Wasn't sure if by "output void *" you meant "returns void *" or "writes to output parameters that are void *" or both, so I tried removing casts for both cases.

Removing casts from a function with output parameters

Change:

void GX_GetFifoPtrs(const GXFifoObj *fifo,void **rd_ptr,void **wt_ptr)
{
	const struct __gxfifo *ptr = (const struct __gxfifo*)fifo;
-	*rd_ptr = (void*)ptr->rd_ptr;
-	*wt_ptr = (void*)ptr->wt_ptr;
+	*rd_ptr = ptr->rd_ptr;
+	*wt_ptr = ptr->wt_ptr;
}

Output:

/home/infra/Projects/libogc/libogc/gx.c: In function 'GX_GetFifoPtrs':
/home/infra/Projects/libogc/libogc/gx.c:1287:17: warning: assignment to 'void *' from 'vu32' {aka 'unsigned int'} makes pointer from integer without a cast [-Wint-conversion]
 1287 |         *rd_ptr = ptr->rd_ptr;
      |                 ^
/home/infra/Projects/libogc/libogc/gx.c:1288:17: warning: assignment to 'void *' from 'vu32' {aka 'unsigned int'} makes pointer from integer without a cast [-Wint-conversion]
 1288 |         *wt_ptr = ptr->wt_ptr;
      |                 ^

Removing cast from a function returning `void *`

Change:

void* GX_GetFifoBase(const GXFifoObj *fifo)
{
-	return (void*)((const struct __gxfifo*)fifo)->buf_start;
+	return ((const struct __gxfifo*)fifo)->buf_start;
}

Output:

/home/infra/Projects/libogc/libogc/gx.c: In function 'GX_GetFifoBase':
/home/infra/Projects/libogc/libogc/gx.c:1477:46: warning: returning 'vu32' {aka 'unsigned int'} from a function with return type 'void *' makes pointer from integer without a cast [-Wint-conversion]
 1477 |         return ((const struct __gxfifo*)fifo)->buf_start;
      |                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~
libogc.a

[endrift]

I had assumed they were void*, not vu32, so my reply was incorrect. @mtheall corrected me. Sorry about the confusion.

[DacoTaco]

@1011X : Hey, sorry for the late reply. Thanks for the PR!
i finally got around testing this PR. i started running all the examples we have in the wii examples repo.
weirdly enough, lesson 2 breaks with this change. can somebody investigate? im not that good with GX nor do i know any graphics stuff :(

the output of the example should be a white square and a triangle according to the lessons its ported from . i confirmed this with the current libogc main. however, im getting a black screen with this change. it didn't crash, as the home button on my wiimote works.

Thanks again!

EDIT : WM might think it has to do with lesson 2 having an Init bug, i might need to test the other lessons and check what i can

[DacoTaco]

little update : lesson 2, 11 & 19 are broken.
however, lesson 19 was already broken and can be ignored.

i do still need somebody to figure out the bug that is now showing up though...

[mardy]

Hi @DacoTaco ! I tried this branch, and it works fine for me: lessons 2 and 11 are working fine for me, and actually lesson 19 too. I'm testing on the Dolphin emulator.

[DacoTaco]

Hi @DacoTaco ! I tried this branch, and it works fine for me: lessons 2 and 11 are working fine for me, and actually lesson 19 too. I'm testing on the Dolphin emulator.

Hey, thanks for testing!

can you test it on a wii as well?
i think it was only broken on actual hardware. i still don't know why, but i don't want to merge this simple PR untill we know why

[mardy]

can you test it on a wii as well? i think it was only broken on actual hardware. i still don't know why, but i don't want to merge this simple PR untill we know why

Ah, indeed! But it may be that WM is right: I now tried to run the original lessons on a wii (via wiiload on TCP) and I'm getting these results:

• lesson2: the first time it loads I always get a blank screen; on the second run it works fine
• lesson 11: I've run this 5 times, and it always hard locks my WII: the screen goes black, but the Home keys is not working, as well as the reset button on the WII. I need to hold the power button for ten seconds to switch off the WII and then restart.
• lesson 19: works reliably.

All this is with the original libogc (I mean, with the version from the official package repo; I haven't try with the master branch). I'll now try with this branch, but it looks like this test is not really going to prove a lot :-)

[DacoTaco]

his is with the original libogc (I mean, with the version from the official package repo; I haven't try with the master branch). I'll now try with this branch, but it looks like this test is not really going to prove a lot :-)

• lesson 11: I've run this 5 times, and it always hard locks my WII: the screen goes black, but the Home keys is not working, as well as the reset button on the WII. I need to hold the power button for ten seconds to switch off the WII and then restart.

if lesson 11 works for you, i might as well finally merge this fix

[mardy]

OK, and here are the results with this branch:

• lesson02: like before, the first time it never works, on the second run it does work (though not all the time! Once I had to run it 4 times to get it display something)
• lesson11: hard lock, as before
• lesson19: works (though once it hard locked the Wii after a few seconds. Then I tried again and it was fine)

Overall, I've the feeling that there's something flacky with the setup of the examples, so I think that this branch is not to blame on the failures.

[DacoTaco]

huh, lesson 19 locked up for me. there is some setup weirdness in the lesson, so ill blame it on that.
merging if its confirmed working for you!