Opened 12 years ago

Closed 10 years ago

#2151 closed defect (fixed)

Small bug in libTIFF on Windows CE

Reported by: fbausch Owned by: Mateusz Łoskot
Priority: normal Milestone:
Component: WinCE Port Version: svn-trunk
Severity: normal Keywords:
Cc: warmerdam

Description

in GDAL, frmts/gtiff/libtiff/tif_dirinfo.c - in the tagCompare routine, which tests inputs a and b for field_tag and field_type match. in the current form, it tests the a parameter (the ta tag) for a wildcard setting (TIFF_ANY), which would allow a simple field_tag match to return success.

but when tagCompare is called in this chain: TIFFFindField - bsearch - tagCompare, it is the b parameter which is specified with the wildcard field (in TIFFFindField); this wildcard is pointless, since only the a parameter is tested for the wildcard setting.

Other call sequences that lead to tagCompare may be different, but in the sequence through TIFFFindField, it is the b parameter which can hold the wildcard type specification, and not the a parameter. the change below fixes the wildcarding operation - other solutions could be to reverse the field parameter order in the call to tagCompare in bsearch (or in TIFFFindField's call to bsearch)

static int tagCompare(const void* a, const void* b) {

const TIFFField* ta = *(const TIFFField) a; const TIFFField* tb = *(const TIFFField) b; /* NB: be careful of return values for 16-bit platforms */ if (ta->field_tag != tb->field_tag)

return (int)ta->field_tag - (int)tb->field_tag;

else

return (ta->field_type == TIFF_ANY) ? 0 : ((int)tb->field_type - (int)ta->field_type);

return (tb->field_type == TIFF_ANY) ? changed return 0 : ((int)tb->field_type - (int)ta->field_type);

}

Change History (6)

comment:1 Changed 12 years ago by fbausch

Component: defaultWinCE Port
Owner: changed from warmerdam to Mateusz Łoskot

on further inspection, bug does not manifest in the windows build, but does manifest in windows CE build. which suggests that the problem is in wce_bsearch, which must be reversing the order of its parameters when compared to the standard bsearch.

which appears to be the case - from wce_bsearch: res = compare(((char*) base + (width * middle)), key); and from bsearch.c in windows: if (!(result = COMPARE(context, key, mid)))

suggested fix is to reverse the wce_bsearch call to compare to: res = compare((key, (char*) base + (width * middle)));

comment:2 Changed 12 years ago by warmerdam

Cc: warmerdam added

comment:3 Changed 12 years ago by fbausch

on further inspection2, suggested fix is inadequate. changing only the order of parameters in the compare line ignores effect of that change in the return value of that operation. suggested fixes with comments in full routine below.

void* wceex_bsearch(const void *key, const void *base, size_t num, size_t width,

int (*compare)(const void *, const void *))

{

size_t left; size_t middle; size_t right; int res;

/* input parameters validation */ assert(key != NULL); assert(base != NULL); assert(compare != NULL);

if (num==0) /* num is unsigned in wce, cannot rely on a comparison to (num-1) */

return NULL;

res = 0; left = 0; right = num - 1;

while (left <= right) {

middle = (left + right) / 2;

res = compare( key, ((char*) base + (width * middle)) ); /* key must be first parameter */

if (res < 0) changed to < from > {

/* search from middle to left */ right = middle - 1;

} else if (res > 0) changed to > from < {

/* search from middle to right */ left = middle + 1;

} else if (res == 0) {

/* middle points to the key element. */ return ((char*) base + (width * middle));

}

}

/* key not found */ return NULL;

}

comment:4 Changed 12 years ago by wbaer

I also had problems with gtiff files on wince since changing from gdal 1.4 to 1.5

I can confirm that the described changes to the wceex_bsearch method are working for me and solve in combination with part 2 of the fixes in #2133 my problems.

Part 1 in #2133 is not needed as the correct fix is described here. Part 3 in #2133 is not needed as already corrected in the further inspection here.

Mateusz please apply the wceex_bsearch changes on wcelibcex and Part2 of #2133 in gdal repository.

comment:5 Changed 12 years ago by Mateusz Łoskot

Status: newassigned
Summary: small bug in libTIFFSmall bug in libTIFF on Windows CE

comment:6 Changed 10 years ago by Mateusz Łoskot

Resolution: fixed
Status: assignedclosed
Version: 1.5.0svn-trunk

bsearch function in wcelibcex rev76 has been fixed - 10 lines patch in 2 years gives coding 5 lines per year - ridiculous me.

Note: See TracTickets for help on using tickets.