The Dink Network

Maybe a bug

September 6th 2005, 12:08 PM
pillbug.gif
Drink
Peasant He/Him Chile
Don't drink 
In Dinkvar.h, "void decompress_nocomp":

if ((c = getc(in)) > 255)
fread(pair,2,c-128,in);
else
{
if (c == '\r') c = '\n';
if (c == 9) c = ' ';

strchar(cbuf,c);
}

in "(c = getc(in)) > 255" it reads one character from a file. "c" is short. Then, c > 255 will never occur, right?

Or there is sense?
September 6th 2005, 01:13 PM
goblins.gif
igloo15
Peasant He/Him
 
May I ask what exactly you are attempting to do with the source. You've asked many questions about different things in the dink source and I was just wondering what you plan to do with it.
September 6th 2005, 01:47 PM
pillbug.gif
Drink
Peasant He/Him Chile
Don't drink 
Oh. First I must be sure that I can finish it.

Mmm. There is no emoticon to say tireness?
December 6th 2005, 01:18 AM
anon.gif
toa
Ghost They/Them
 
Shorts range from 32767 to -32768 so it's not a bug.

However, since the characters returned by 'getc' only range from 0 to 255 (unsigned char casted to int) and the error condition is -1 (EOF), the true condition will never be executed because 'c' will never be greater than 255.

Furthermore, 'strchar' in this context is a painter's error -- better to rewrite it, keeping track of the end of cbuf:

Old: strchar(cbuf,c);
New: cbuf[i++] = c; cbuf[i] = '\0';
December 6th 2005, 02:37 AM
dinkdead.gif
millimeter
Peasant He/Him Canada
Millimeter is Wee-Lamm, Recording Artist. :-) 
I disagree.

if ((c = getc(in)) > 255)
//if not ascii char then c = # of
//char to read.

fread(pair,2,c-128,in);
//pair = *ptr to pair table
//2 = 2 char per set
//c-128 = # of char/2
//in = stream (script file)

else
{
if (c == '\r') c = '\n';
//if EOL end of line, do Carriage Return also.
if (c == 9) c = ' ';
//If tab, make it a single space
strchar(cbuf,c);
//Must be char. so load buffer with
//c = # of char to read.
}

so if c>255 then read_a_number_of_pairs
else CRLF
else Whitespace
else read to buffer size.

if c was intended to be only byte sized, then it would have been a waste to code it as a 2 byte address, considering the platform this was written to.

hth,
mm.
December 6th 2005, 10:48 AM
anon.gif
toa
Ghost They/Them
 
Disagree!?

Code is code. There is no opinion, Luke, only right or right-not.

"if c was intended to be only byte sized, then it would have been a waste to code it as a 2 byte address, considering the platform this was written to."

C is the character already read. If C >= 128, it's the number of pair definitions following in the file. ((c-128)*2 bytes in the table)

C is NOT intended to be byte-sized. C, as returned from getc(), is an int (which can be anywhere from sizeof(char) bytes to 523MB, depending on the hardware). The reason it's returned as an int is to allow a full range of character codes AND a sentinal value to singal error conditions. If it is declared short, that's acceptable only because it allows the same range and sentinal value.

And the "> 255" will always return false. In the other function (which handles decompression), it's done correctly. (If I remember correctly: "> 127".) Apparantly, Seth was lazy and just changed one number to kill unwanted code instead of actually removing it.
December 6th 2005, 02:21 PM
anon.gif
millimeter
Ghost They/Them
 
Easy now. I agree that as a peasant, I am new to this forum and I do seem to question many coding techniques. Perhaps Luke is a little harsh, but only time will tell whether I am adding positive dialog or merely whining, as you suggest.

My read of the functions are
if script.d then use decompress.
if script.c then use decompress_nocomp.
the 2 are essentially the same except for this fact and the value of c(0).

if ((c = getc(in)) > 255)
fread(pair,2,c-128,in);
else

This says clearly,
if c > 255 then fread(pair,2,c-128,in)
// Parse in pairs
if c =< 255 then else
// Parse in single (ascii) characters

C is the character already read. If C >= 128, it's the number of pair definitions following in the file. ((c-128)*2 bytes in the table)
C is NOT intended to be byte-sized. C, as returned from getc()...


I believe that is what I said in answer to Drink's original question.
in "(c = getc(in)) > 255" it reads one character from a file. "c" is short. Then, c > 255 will never occur, right?

If c > 255 always returned false as you suggest, then the function would never execute the
fread(pair,2,c-128,in); and would never acknowledge the pairs table, if one existed. Everything in cbuf would be only tested for CR/LF/Whitespace or parsed as text.

If a non-ascii char was then found in cbuf, it would generate an error which has not been dealt with. However, by testing to see if a pairs table existed, then the non-ascii value would be treated as data value rather than text.

imho, Seth was not merely killing code he used the 2 functions to allow compiled.D and non-compiled.C scripts to both be usable. This should also prove out that script.D should take precedence over script.C, but this gets far away from Drink's question.

I stand that c > 255 is NOT a bug, but this is how Seth is testing for the existence of a pairs table.
mm

December 7th 2005, 03:48 AM
anon.gif
toa
Ghost They/Them
 
The test will always return false. Think about it:

It's not a compressed file; why would there be a pair table in it?

Non-ascii characters in cbuf aren't an error. Try "say"ing something with non-ascii chars. Betcha 20 euros it works.

"This should also prove out that script.D should take precedence over script.C,"

Precedence is handled by suffix checking, i.e., the first file tested for existance is the ".d" and, if found, ".c" is not checked for.

Also: I didn't mean to be read as harsh. I just have an emoticon disability.
December 7th 2005, 07:57 AM
seth.gif
Seth
Peasant He/Him Japan
 
toa is correct.

IIRC I stole that byte-pair encoding/unencoding source originally and at the last minute made a hacked version that could handle upper-ascii because we were doing a german version and the umlauts and whatnot were causing problems.

Looking at it now, yeah, looks like the entire check should be removed, not needed.
December 7th 2005, 10:58 PM
anon.gif
millimeter
Ghost They/Them
 
Thank you Seth for setting me straight.
I shall re-check my Luke level.
December 7th 2005, 11:02 PM
anon.gif
millimeter
Ghost They/Them
 
Also: I didn't mean to be read as harsh. I just have an emoticon disability

<humility>
Accordingly my ignorance/arrogance would have warranted it, but you were not harsh...just right.
</humility>
mm
December 8th 2005, 12:19 AM
anon.gif
toa
Ghost They/Them
 
"but you were not harsh...just right."

Won't post unless I am.

And don't worry about that "ignorance/arrogance" thing. Admitting you're wrong is the first step to wisdom and higher education.