Skip to content

Fix gigimport in RSA#11

Merged
JayFoxRox merged 1 commit into
XboxDev:masterfrom
JayFoxRox:fix-gigimport
Apr 8, 2020
Merged

Fix gigimport in RSA#11
JayFoxRox merged 1 commit into
XboxDev:masterfrom
JayFoxRox:fix-gigimport

Conversation

@JayFoxRox
Copy link
Copy Markdown
Member

@JayFoxRox JayFoxRox commented Jun 7, 2019

Backport of changes that were done in https://github.com/Cxbx-Reloaded/Cxbx-Reloaded/pull/1561

Additionally, an assert was removed because it warns about (what appears to be) a valid state. See this discussion on this PR: #11 (comment)

Hopefully closes #9 and hopefully closes #10

Comment thread xboxlib.c Outdated
g->sign -= 1;
}

assert(g->sign != 0);
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This assert gets triggered for nxdk XBEs now, because buff is len bytes of zero.

I'm not sure why this check exists in the first place?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This seems to be a hack that was introduced here: dd70311#diff-e64a6c66ddd61d0fa1f0670ca3423da2L227


I don't think the additional check / fixup for g->sign == 0 isn't even necessary, because

xbedump/giants.c

Lines 359 to 377 in d0d6508

int isZero(
giant thegiant
)
/* Returns TR if thegiant == 0. */
{
register int count;
int length = abs(thegiant->sign);
register unsigned short * numpointer = thegiant->n;
if (length)
{
for(count = 0; count<length; ++count,++numpointer)
{
if (*numpointer != 0 )
return(FA);
}
}
return(TR);
}
also checks if the length is zero. So g->sign == 0 appears to be a valid state.

It also handles 0 like that in this:

xbedump/giants.c

Lines 393 to 407 in d0d6508

void
itog(
int i,
giant g
)
/* The giant g becomes set to the integer value i. */
{
unsigned int j = abs(i);
if (i==0)
{
g->sign = 0;
g->n[0] = 0;
return;
}

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I've removed this assert.

@chrisderwahre
Copy link
Copy Markdown
Contributor

I tested this PR on the following:

  • 2 Retail Games [XDK Binaries]
  • 2 NXDK Binaries

All worked just fine after this change.
Before these changes, NXDK binaries would seg fault. [As this PR also says that it fixes #10 ]

@GXTX
Copy link
Copy Markdown

GXTX commented Apr 7, 2020

Tested using the hello sample from nxdk and can confirm it does work as expected. Hopefully we can get another maintainer to verify and merge, thanks.

@JayFoxRox JayFoxRox merged commit 4ff51a9 into XboxDev:master Apr 8, 2020
@JayFoxRox JayFoxRox deleted the fix-gigimport branch April 8, 2020 15:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Segmentation fault on NXDK Xbe's Wrong giant.sign from gigimport

3 participants