Bug in /CharInfo

Need help running MacroQuest 1? Too bad! Use MQ2.

Moderator: MacroQuest Developers

Mckorr
Developer
Developer
Posts: 2326
Joined: Fri Oct 18, 2002 1:16 pm
Location: Texas

Post by Mckorr » Fri Mar 21, 2003 5:25 pm

Yes, by ASM I mean Assembly. Sorry.

And yes, I'm using my source code version, but the only major difference between my code and the official version is the removal of PERL. Beyond that I just altered the #include paths to point to my restructured source code tree:

Code: Select all

SRC-20030308-MCK
     EQLib
          include
          lib
          resource
          release
     MacroQuest
          include
          lib
          resource
          release
     Release
It could be the no-PERL mod I'm using. Since I never used the PERL functions I edited them out with Valerian's help. Didn't see any reason to keep them and the extra files needed to run them.

When I get the time (mother-in-law in town, redecorating the living room, doing a ton of yard work outside, work) I'll have to take a look and see if your changes make a call to anything using PERL.

azwildfire
Contributing Member
Contributing Member
Posts: 72
Joined: Fri Jul 05, 2002 5:12 am
Location: Phoenix arizona
Contact:

Post by azwildfire » Fri Mar 21, 2003 5:38 pm

Greetings
thank you for the fix about guild tags
however when i tried to adjust this fix, i added it in, and tripple checked and i still crash when doing /charinfo

any info i can give you to help?
perhaps there was another change somewhere you did instinctively and missed in your above steps?

would you like me to send you my MQ.h file and EQLib.ccp to double check that i have implimented your code correctly?

not a big dela to me at all i am happy to have guild tags back, just though i would give this a shot also

Cheers!

Mckorr
Developer
Developer
Posts: 2326
Joined: Fri Oct 18, 2002 1:16 pm
Location: Texas

Post by Mckorr » Fri Mar 21, 2003 5:51 pm

Forgive me for saying so, but glad I'm not the only one =)

icon
Official loudmouth
Official loudmouth
Posts: 158
Joined: Fri Jun 14, 2002 2:59 pm
Location: ...
Contact:

Post by icon » Fri Mar 28, 2003 8:46 pm

I may be way off, but when you have an array such as bankslots[16] or whatever, aren't you supposed to use one less then common sense would tell you? Like use bankslots[15] instead of 16 because bankslots[0] through bankslots[16] would be an actual 17 different slots? array-wise I mean?

Did that make sense? I just woke up.

- Icon
In memory of [b][color=darkblue]MasTerKeyZ[/b][/color].

NealThorpayt
Developer
Developer
Posts: 66
Joined: Thu Mar 13, 2003 2:14 pm
Location: Miskatonic University
Contact:

Post by NealThorpayt » Fri Mar 28, 2003 10:01 pm

Greetings Construcs,
I may be way off, but when you have an array such as bankslots[16] or whatever, aren't you supposed to use one less then common sense would tell you?
No, you are NOT way off. In fact you are correct. However, It makes only a small amount of difference in this case. As the bank information is last in the CHARINFO struct. If there were anything to follow the bank information, having the array size equal to 16 would be a problem. And, if you look at the code that checks the actual bank information, it has a bounds check of '<' (less than) which makes it go to one less than the actual value of NUM_BANK_SLOTS.

In any case, the more proper code would include a change to the CHARINFO struct where the bankslot array would be rewritten to:

Code: Select all

 PITEMINFO   Bank[NUM_BANK_SLOTS-1]; 
Thank you for catching this.

End of line...
By the pricking of my thumb, something wicked this way comes...

User avatar
dont_know_at_all
Developer
Developer
Posts: 5450
Joined: Sun Dec 01, 2002 4:15 am
Location: Florida, USA
Contact:

Post by dont_know_at_all » Fri Mar 28, 2003 10:21 pm

Uh, no.

Code: Select all

int bankslot[16], i;

for(i=0;i<16;i++) {
    if (bankslot[i]) {
        // etc

The declaration has the correct number. The indices go from 0 to n-1.

NealThorpayt
Developer
Developer
Posts: 66
Joined: Thu Mar 13, 2003 2:14 pm
Location: Miskatonic University
Contact:

Post by NealThorpayt » Fri Mar 28, 2003 11:00 pm

Greetings Constructs,

UH NO!!! Your post changes only the loop from a #define to a numeric constant. While this is technically correct it can lead to problems down the line.
Uh, no.

Code:

int bankslot[16], i;

for(i=0;i<16;i++) {
if (bankslot) {
// etc




The declaration has the correct number. The indices go from 0 to n-1.


Read what I posted. The change is only in the CHARINFO struct. NOT in the loop!

End of line...
By the pricking of my thumb, something wicked this way comes...

User avatar
dont_know_at_all
Developer
Developer
Posts: 5450
Joined: Sun Dec 01, 2002 4:15 am
Location: Florida, USA
Contact:

Post by dont_know_at_all » Sat Mar 29, 2003 12:52 am

Making it one less than 16 implies that it's size is 15 when in fact the size of the array is 16. If they put something after the bank slots, 15 will be wrong. That will be a problem down the line.

There are now 16 bank items in the charinfo struct. The fact that we correctly access indices 0 to 15 proves this.

NealThorpayt
Developer
Developer
Posts: 66
Joined: Thu Mar 13, 2003 2:14 pm
Location: Miskatonic University
Contact:

Post by NealThorpayt » Sat Mar 29, 2003 1:05 pm

Greetings Constructs,

<bows> I offer my apologies to DKAA...

He is absolutely correct. The current definition of the array bounds is correct.

I should keep from posting late at night after having a few cocktails.

End of line...
By the pricking of my thumb, something wicked this way comes...

icon
Official loudmouth
Official loudmouth
Posts: 158
Joined: Fri Jun 14, 2002 2:59 pm
Location: ...
Contact:

Post by icon » Mon Mar 31, 2003 1:01 am

Maybe I'm dumb today, as I've been very sick for the whole weekend, but DKIA's post confused me a bit.

"Making it one less than 16 implies that it's size is 15 when in fact the size of the array is 16."

So, you are really saying, "When you declare bankslots[16] the [16] is only declaring it a size of 16 and the actual place 'bankslots[16]' would be invalid there."?

What I got from what you said is that when I declare an array of x[y], y is only the SIZE of the array and NOT what x[y] goes up to? Like with bankslots[16]... you are saying that if bankslots[15] was declared instead, the actual place of bankslots[15] could not be accessed, as bankslots[14] in that case would BE the 15th place?

This is hard to express.

Basically, what I think you are saying, is that when we declare bankslots[16] we are actually only declaring bankslots[0] through bankslots[15]? And 16 would still be invalid?

Let me know if my dim-witted assumptions are correct.

- Icon
In memory of [b][color=darkblue]MasTerKeyZ[/b][/color].

User avatar
dont_know_at_all
Developer
Developer
Posts: 5450
Joined: Sun Dec 01, 2002 4:15 am
Location: Florida, USA
Contact:

Post by dont_know_at_all » Mon Mar 31, 2003 1:09 am

icon wrote:So, you are really saying, "When you declare bankslots[16] the [16] is only declaring it a size of 16 and the actual place 'bankslots[16]' would be invalid there."?
In a word: yes.

Think of the case of an array with one entry.

int data[1];

The compiler reserves space for one int, in our case 4 bytes.

The only "valid" index into this array is 0 since that is the first index in C arrays.

icon
Official loudmouth
Official loudmouth
Posts: 158
Joined: Fri Jun 14, 2002 2:59 pm
Location: ...
Contact:

Post by icon » Mon Mar 31, 2003 1:25 am

Ahh, got it, thanks! 8)

That explains the for loop you posted above,

int bankslots[16], i;

for(i=0;i<16;i++)
{
do.something -> bankslots;
}

So in this example the loop would only run 16 times (i==0 through i==15), incrementing once each time, and using bankslots[0] through bankslots[15], and never actually attempting to use bankslots[16] since the [16] would only be valid if you declared size of 17.

In other words, I got it now, thanks!

- Icon
In memory of [b][color=darkblue]MasTerKeyZ[/b][/color].

azwildfire
Contributing Member
Contributing Member
Posts: 72
Joined: Fri Jul 05, 2002 5:12 am
Location: Phoenix arizona
Contact:

Post by azwildfire » Wed Apr 02, 2003 9:14 am

so does anyone have the new code to post? or is this still a bug? i can't follow all this mumbo jumbo very well :P

and the release of the eqlib.cpp file in the CSV area does not have the guild tag fix in it. in the who, anyone with a guild name just has <> by thier name...

Cheers!