Thread: Increment gone wrong

Originally Posted by hamster_nz

I tried this - output is wrong too:

[code]
$ echo "aaaaaabbbbbaaaaacccccdddddd" | gzip > a.gz

$ ./a.out --verbose -f a.gz
main.c:28: ArgHasIssue( '--verbose', '(null)' )
main.c:9: ArgHasIssue( '-f', 'a.gz' )
LoadGzipDetails( 0x7ffff68bc7e0, 28 )
Gzip Header: magic = 1F8B, format = 08, flags = 00000000, xflags = 00, system = 03
Flag States: TEXT = false, HCRC = false, MORE = false, NAME = false, NOTE = false, RESERVED = 0
Expected Size = 28
gzip.c:128: Stream Details: ptr = 0x7ffff68bc7e0, byte = 33, took = 264, used = 80, have = 232
Zlib Expansion Result:
aaaaaa

Looked into it, turned out there was a left over break statement from when I split up the code from it's huge block in main() during early stages, that break was meant to be for code 256 only, went and moved it while I was implmenting support for the code again, I really hope thats the last issue in the deflation algorithm

This is the closest thread topic to my newest problem, I finally realised why I've been failing to extract the right raw image from png files, turns out I misunderstood the IDAT chunk, after correcting that I found that my bit stream function is faulty, it somehow works for "<= bitsof(uintmax_t) " (to be accurate I've only read upto 32 bits at a time so far) but as soon as it was need for a larger block it resulted in it the bits being ignored, here's the code for the stream functions 1st:

Code:

void StreamUsed( STREAM *Stream, uint bits )
{
	intmax_t max = BUFSIZ;
	max *= CHAR_MAX;
	Stream->took += bits;
	Stream->used += bits;

	if ( Stream->eod && Stream->took >= max )
		Stream->err = EOF;
}

long StreamBlock( STREAM * Stream )
{
	void *src = Stream->feed;
	long add = -1;

	memset( src, 0, BUFSIZ );

	if ( Stream->eod )
		Stream->err = EOF;
	else
	{
		add = Stream->DataCB( src, Stream->data, Stream->byte, BUFSIZ );
		Stream->eod = (add < BUFSIZ);
	}

	return add;
}

int StreamData( STREAM *Stream, void *dst, intmax_t get, bool use )
{
	BIT d, s;
	intmax_t const max = ((intmax_t)BUFSIZ) * CHAR_BIT;
	intmax_t num = max - Stream->took;
	void * src = Stream->feed;

	SetBit( &d, dst, 0 );

	if ( Stream->byte < 1 )
	{
		Stream->took = 0;
		num = StreamBlock( Stream );

		if ( num < 0 )
			return EOF;

		if ( !num )
			return ENODATA;

		Stream->byte = num;
		num *= CHAR_BIT;
	}

	while ( d.abs < get )
	{
		intmax_t take = get - d.abs, bits = Stream->took;

		while ( bits >= max )
		{
			num = StreamBlock( Stream );

			if ( num < 0 )
				return EOF;

			if ( !num )
				return ENODATA;

			Stream->byte += num;
			num *= CHAR_BIT;
			bits -= max;
			Stream->took = 0;

			EchoStreamDetails( stdout, Stream );
		}

		SetBit( &s, src, Stream->took );

		if ( num < take )
			take = num;

		for ( bits = 0; bits < take; ++bits )
		{
			if ( *(s.byte) & s.mask )
				*((uchar*)(d.byte)) |= d.mask;

			IncBit( &d, dst );
			IncBit( &s, src );
		}

		if ( use )
			StreamUsed( Stream, take );
		else if ( get > num )
			return ERANGE;
	}

	return 0;
}

And here's what is being passed to it when I noticed it was ignoring the bits:

Code:

		else if ( ansi_cmpi( Entry->Type, "IDAT" ) == 0 )
		{
			uint cmf = StreamBits( Stream, 8, true );
			uint mod = StreamBits( Stream, 8, true );
			uint chk;

			idat = ExpandBuffer( IDAT, 1, IDAT->used + leng );

			if ( !idat )
			{
				fprintf( errout, "Not enough space to append IDAT data\n" );
				return NULL;
			}

			leng -= 4;

			err = StreamData( &StreamFile, idat + IDAT->used, leng * 8, true );

			chk = StreamBits( &StreamFile, 8, true ) << 8;
			chk |= StreamBits( &StreamFile, 8, true );
			IDAT->used += leng;

			if ( verbose )
			{
				EchoStreamDetails( verbose, &StreamFile );
				ECHO
				(
					verbose,
					fprintf
					(
						verbose,
						"cmf = %02X, mod = %02X, chk = %04X, "
						"leng = %5" PRIuMAX ", IDAT->used = %7u\n",
						cmf, mod, chk, leng, IDAT->used
					)
				);
				EchoStreamDetails( verbose, &StreamFile );
			}

			if ( err )
			{
				ECHO( errors, ECHO_ERR( errors, err ) );
				return NULL;
			}
		}

Local Output:

Code:

view/png.c:309: 2 'IDAT' 111
Stream Details: End Of Data =  true, End Of Stream = false, data = 0x7ffc4e9fd188, byte = 184, took = 1328, used = 1328
view/png.c:356: cmf = 00, mod = 00, chk = 5220, leng =   107, IDAT->used =     107
Stream Details: End Of Data =  true, End Of Stream = false, data = 0x7ffc4e9fd188, byte = 184, took = 1328, used = 1328

That 1st line in the output represents chunk number, chunk type and the leng variable you see mentioned in the code (leng of the chunk), anyone have any ideas what's causing StreamData() to ignore the bits it's supposed to be putting in "idat"?

Edit: Never mind, failed to notice that I was only printing after the readout, moving the first printout to before the readout gave me the expected output for the details, didn't resolve my issue with IDAT somehow being read wrong though.

Edit 2: If anyone was planning to help me understand what's going wrong with the IDAT readout the current upload is here:
Files * d700e396122a99a8ab07a489accbb5596abd6c49 * Lee Shallis / glEngine * GitLab
Look for png.c then LoadImagePNG(), then just look for the if statement that compares the entry type to the string IDAT, that should be the highest code statement you need to look at, stream code is in stream.h/stream.c, BIT is defined in common.h (forgive the somewhat poorly organised code, my focus has been getting the images extracted right and learning opengl, cleaner versions will eventually be put in paw with minimal header dependencies)

I think I already fixed that one, probably didn't copy it over to the original code, was just a lack of support for allocating memory at that point I think, I'll go ahead and copy that over now before going to bed

Edit: Here's the upload with the later fixes included:

deflate * 821d8f1fcada79c0dd4bdbf65eefd16bbe8bdb34 * Lee Shallis / UC-Evidence * GitLab

Originally Posted by hamster_nz

Pulled those changes down, rebuilt and retried. Same result.

Code:

$ gzip < main.c > a.gz
$ ./a.out -f a.gz
zlib.c:820: last = 1, type = 10, Stream->used = 83
zlib.c:318: Couldn't find symbol!
Symbols List Details: foresee:  14, longest =  6, highest = 64
Error 0x00000016 (22) 'Invalid argument'
main.c:197: Error 0xFFFFFFFF (-1) 'Unknown error -1'

After adding some print statements to track the results of tables I found it's not the reading of the encoding thats the issue, its the point that I move each symbol to an index that matches it's uid (for speed during the deflation process):

Code:

typedef struct _ZLIB_SYMBOL
{
	uint uid;
	uint src;
	uint val;
	uint sym;
	uint lit;
	uint len;
	uint get;
	uint cpy;
	uint use;
	uint nxt[2];
} ZLIB_SYMBOL;
...
int CompareSymbols( void const *A, void const *B )
{
	ZLIB_SYMBOL const *a = A, *b = B;

	if ( a->use != b->use )
		return b->use - a->use;

	if ( a->len != b->len )
		return a->len - b->len;

	return a->uid - b->uid;
}
...
int SortZlibSymbolsByUID( ZLIB *zlib, int id )
{
	uint j = 0;
	ZLIB_SYMBOLS *Symbols = &(zlib->Symbols[id]);
	BUFFER *Entries = AccessBuffer( zlib->Buffers, Symbols->entries );
	ZLIB_SYMBOL *symbols = Entries->addr, *dst, *src;
	FILE *verbose = zlib->Buffers->Alloc->verbose;

	if ( verbose && !quiet_zlib )
	{
		ECHO
		(
			verbose,
			fprintf( verbose, "Prior to qsort() " );
			EchoZlibSymbolsListDetails( verbose, zlib, id )
		);
	}

	qsort( symbols, Entries->used, sizeof(ZLIB_SYMBOL), CompareSymbols );

	if ( verbose && !quiet_zlib )
	{
		ECHO
		(
			verbose,
			fprintf( verbose, "Prior to manual " );
			EchoZlibSymbolsListDetails( verbose, zlib, id )
		);
	}

	symbols = GrowZlibSymbolsBuffer( zlib, id, Symbols->highest + 1 );

	if ( !symbols )
		return ENOMEM;

	for ( j = Entries->used - 1; j; --j )
	{
		src = symbols + j;
		dst = symbols + src->uid;

		if ( src->uid > j )
		{
			*dst = *src;
			/* This is where I'm loosing data when I shouldn't be (in theory anyways) */
			memset( src, 0, sizeof(ZLIB_SYMBOL) );
		}
	}

	Entries->used = Symbols->highest + 1;

	return 0;
}

I've added a comment to the point I can see is causing the error down the line, although in theory shouldn't be, any ideas on how to fix it? Or even better any ideas on how to fob off the duty to CompareSymbols()?

Welp, just tried this method instead:

Code:

	for ( j = Entries->used - 1; j; --j )
	{
		ZLIB_SYMBOL tmp;
		src = symbols + j;
		dst = symbols + src->uid;
		tmp = *src;
		memset( src, 0, sizeof(ZLIB_SYMBOL) );
		*dst = tmp;
	}

Thought maybe forcing the data back in after clearing src might fix the issue, it did not.

After further investigation it turned out to be right before the manual sort, during the allocation, since the GrowZlibSymbolsBuffer() just defers to ExpandBuffer() I'll skip to that one instead:

Code:

void * ExpandBuffer( BUFFER *Buffer, int nsize, int nwant )
{
	ALLOC *Alloc = Buffer->Alloc;
	uchar *tmp = Buffer->addr;
	size_t get, had = Buffer->size;

	nwant = (nwant > 0) ? nwant : 0;
	get = nwant * nsize;

	if ( nsize < 1 )
		return NULL;

	if ( get <= had )
		return tmp;

	tmp = Alloc->alloc( Alloc->ud, tmp, had, get );

	if ( !tmp )
		return NULL;

	Buffer->addr = tmp;
	Buffer->size = get;
	Buffer->have = get / nsize;

	return tmp;
}
/* Just in case this is what Alloc->alloc points to atm */
void * default_allocator( void *ud, void *ptr, size_t had, size_t get )
{
	uchar *tmp;
	(void)ud;

	if ( !get )
	{
		if ( ptr )
			free( ptr );

		return NULL;
	}

	tmp = ptr ? malloc( get ) : realloc( ptr, get );

	if ( !tmp )
		return NULL;

	if ( get > had )
		memset( tmp + had, 0, get - had );

	return tmp;
}

Originally Posted by awsdert

After adding some print statements to track the results of tables I found it's not the reading of the encoding thats the issue, its the point that I move each symbol to an index that matches it's uid (for speed during the deflation process):

Code:

typedef struct _ZLIB_SYMBOL
{
    uint uid;
    uint src;
    uint val;
    uint sym;
    uint lit;
    uint len;
    uint get;
    uint cpy;
    uint use;
    uint nxt[2];
} ZLIB_SYMBOL;
...
int CompareSymbols( void const *A, void const *B )
{
    ZLIB_SYMBOL const *a = A, *b = B;

    if ( a->use != b->use )
        return b->use - a->use;

    if ( a->len != b->len )
        return a->len - b->len;

    return a->uid - b->uid;
}
...
int SortZlibSymbolsByUID( ZLIB *zlib, int id )
{
    uint j = 0;
    ZLIB_SYMBOLS *Symbols = &(zlib->Symbols[id]);
    BUFFER *Entries = AccessBuffer( zlib->Buffers, Symbols->entries );
    ZLIB_SYMBOL *symbols = Entries->addr, *dst, *src;
    FILE *verbose = zlib->Buffers->Alloc->verbose;

    if ( verbose && !quiet_zlib )
    {
        ECHO
        (
            verbose,
            fprintf( verbose, "Prior to qsort() " );
            EchoZlibSymbolsListDetails( verbose, zlib, id )
        );
    }

    qsort( symbols, Entries->used, sizeof(ZLIB_SYMBOL), CompareSymbols );

    if ( verbose && !quiet_zlib )
    {
        ECHO
        (
            verbose,
            fprintf( verbose, "Prior to manual " );
            EchoZlibSymbolsListDetails( verbose, zlib, id )
        );
    }

    symbols = GrowZlibSymbolsBuffer( zlib, id, Symbols->highest + 1 );

    if ( !symbols )
        return ENOMEM;

    for ( j = Entries->used - 1; j; --j )
    {
        src = symbols + j;
        dst = symbols + src->uid;

        if ( src->uid > j )
        {
            *dst = *src;
            /* This is where I'm loosing data when I shouldn't be (in theory anyways) */
            memset( src, 0, sizeof(ZLIB_SYMBOL) );
        }
    }

    Entries->used = Symbols->highest + 1;

    return 0;
}

I've added a comment to the point I can see is causing the error down the line, although in theory shouldn't be, any ideas on how to fix it? Or even better any ideas on how to fob off the duty to CompareSymbols()?

Your problem is in here, from "buffer.c".

Code:

#include "memory.h"

void * default_allocator( void *ud, void *ptr, size_t had, size_t get )
{
        uchar *tmp;
        (void)ud;

        if ( !get )
        {
                if ( ptr )
                        free( ptr );

                return NULL;
        }

        tmp = ptr ? malloc( get ) : realloc( ptr, get );

        if ( !tmp )
                return NULL;


        if ( get > had )
                memset( tmp + had, 0, get - had );


        return tmp;
}
...

Originally Posted by hamster_nz

Your problem is in here, from "buffer.c".

Code:

#include "memory.h"

void * default_allocator( void *ud, void *ptr, size_t had, size_t get )
{
        uchar *tmp;
        (void)ud;

        if ( !get )
        {
                if ( ptr )
                        free( ptr );

                return NULL;
        }

        tmp = ptr ? malloc( get ) : realloc( ptr, get );

        if ( !tmp )
                return NULL;


        if ( get > had )
                memset( tmp + had, 0, get - had );


        return tmp;
}
...

I'm not seeing the cause, if you've already seen it then enlighten me please

Originally Posted by hamster_nz

I assume you found it, and just didn't bother replying?

Just didn't notice the message until just now, wasn't expecting a fast response since I figured you were probably doing something else and just checking occasionally like I do

Originally Posted by hamster_nz

...

Code:

        if ( get > had )
                memset( tmp + had, 0, get - had );

...

are you thinking this is the cause, I don't see how as that only touches new memory, not old memory