#172 ilInit() does not clear the error buffer

open
nobody
None
5
2009-06-03
2009-06-03
Ron
No

lInit() and ilShutDown() do not clear the error buffer. It seems like ilInit() definitely should do so to ensure that the library begins in a pristine state.

Here's my C# example case:
Il.ilInit();
int type = Il.ilTypeFromExt(null); //adds an error 1289, invalid param, to the DevIl error stack
Il.ilLoadL(type, readBytes, readSize);
Il.ilShutDown();

Il.ilInit();

// there's an error on the stack from the earlier call to ilTypeFromExt()!
int error = Il.ilGetError();
if (error != Il.IL_NO_ERROR)
throw new InvalidOperationException(string.Format("DevIl error {0}", error));

I looked through the DevIl source code and I think I see the problem.
ilInit() first calls ilSetError(IL_NO_ERROR).
The intention seems to be that ilInit() will clear the error stack, but that's not what happens. IL_NO_ERROR is simply placed on the stack, which I think is mistaken behavior for ilSetError.
ilInit() then calls ilDefaultStates() which attempts to clear the error stack by repeatedly calling ilGetError(). This loop will be executed once, leaving the error stack with any previous errors on it!

I recommend that ilSetError() clear the error stack when the error parameter is IL_NO_ERROR. Additionally, I think il_error should use a circular buffer so that the 'for' loop in ilSetError could be removed. If there are thousands or tens of thousands of errors, that 'for' loop each time can start to add up as a performance penalty.

Here is my suggested fix:
//-----------------------------------------------------------------------------
//
// ImageLib Sources
// Copyright (C) 2000-2008 by Denton Woods
// Last modified: 06/02/2007
//
// Filename: src-IL/src/il_error.c
//
// Description: The error functions
//
//-----------------------------------------------------------------------------

#include "il_internal.h"

#define IL_ERROR_STACK_SIZE 32 // Needed elsewhere?

ILenum ilErrorNum[IL_ERROR_STACK_SIZE]; //a circular buffer of error codes
ILint ilNumErrors = 0; //number of errors currently in the circular buffer
ILint ilNextIndex = 0; //the index to place the next error in the circular buffer

// Sets the current error.
// If you go past the stack size for this, the oldest errors are lost, like a LRU algorithm.
// If you pass in IL_NO_ERROR, all the errors will be cleared.
ILAPI void ILAPIENTRY ilSetError(ILenum Error)
{
if (Error == IL_NO_ERROR) {
ilNumErrors = 0;
return;
}

ilErrorNum[ilNextIndex] = Error;

ilNextIndex++;
if (ilNextIndex == IL_ERROR_STACK_SIZE)
ilNextIndex = 0;

if (ilNumErrors < IL_ERROR_STACK_SIZE)
ilNumErrors++;

return;
}

//! Gets the last error on the error stack, popping it from the stack.
// If the error stack is empty, IL_NO_ERROR is returned.
ILenum ILAPIENTRY ilGetError(void)
{
if (ilNumErrors > 0) {

ilNumErrors--;
if (--ilNextIndex < 0)
ilNextIndex = IL_ERROR_STACK_SIZE - 1;

return ilErrorNum[ilNextIndex];
}

return IL_NO_ERROR;
}

Discussion

  • Ron
    Ron
    2009-06-03

    the source code got mangled by the bug text form. Here's the file.

     
    Attachments