AWARE [SYSTEMS] Imaging expertise for the Delphi developer
AWare Systems, Imaging expertise for the Delphi developer, Home TIFF and LibTiff Mailing List Archive

LibTiff Mailing List

TIFF and LibTiff Mailing List Archive
February 2012

Previous Thread
Next Thread

Previous by Thread
Next by Thread

Previous by Date
Next by Date

Contact

The TIFF Mailing List Homepage
This list is run by Frank Warmerdam
Archive maintained by AWare Systems



Valid HTML 4.01!



Thread

2012.02.28 19:56 "Confusion in implementing _TIFFVGetField, adding a custom TAG", by <edwardswangren@gmail.com>
2012.02.28 22:02 "Re: Confusion in implementing _TIFFVGetField, adding a custom TAG", by <edwardswangren@gmail.com>

2012.02.28 19:56 "Confusion in implementing _TIFFVGetField, adding a custom TAG", by <edwardswangren@gmail.com>

I'm new to libtiff, so I want to make sure that my assumptions are correct,
but I believe that I have stumbled across an obvious issue in the library.

I am adding my own built-in tags to the library using the documentation
here: http://libtiff.maptools.org/addingtags.html

So, I added an entry to the TIFFFieldInfo array defined at the top of
tif_dirinfo.c like so:

(not sure if code tags work here...)
[code]
{ TIFFTAG_CUSTOM_XXX, 4, 4, TIFF_SLONG, FIELD_XXX,
1, 0, "XXX" },
[/code]

I then added a field to the TIFFDirectory structure defined in tif_dir.h:

[code]
int32 td_xxx[4];
[/code]

Now I went ahead and modified _TIFFVSetField and _TIFFVGetField as
instructed.  This is where I hit a problem.

In mimicking the pattern already present in the library, I added the
following code to _TIFFVGetField:

[code]
case TIFFTAG_CUSTOM_SCANCOORDMICRONS:
	*va_arg(ap, int32*) = td->td_xxx[0];
	*va_arg(ap, int32*) = td->td_xxx[1];
	*va_arg(ap, int32*) = td->td_xxx[2];
	*va_arg(ap, int32*) = td->td_xxx[3];
	break;
[/code]

As far as I can tell this is completely incorrect.  For reference please
look to the section of the switch which deals with TIFFTAG_YCBCRSUBSAMPLING.

This is analogous to what I am doing, i.e., populate the input based off of
an array of ints, the only difference being the size of the data type.

_TIFFVGetField is eventually called from TIFFWriteNormalTag in
tif_dirwrite.c.  The relevant code is this:

[code]
case TIFF_LONG:
	case TIFF_SLONG:
	case TIFF_IFD:
		if (fip->field_passcount) {
			uint32* lp;
			if (wc == (uint16) TIFF_VARIABLE2) {
				TIFFGetField(tif, fip->field_tag, &wc2, &lp);
				TDIRSetEntryCount(tif,dir, wc2);
			} else {	/* Assume TIFF_VARIABLE */
				TIFFGetField(tif, fip->field_tag, &wc, &lp);
				TDIRSetEntryCount(tif,dir, wc);
			}
			if (!TIFFWriteLongArray(tif, dir, lp))
				return 0;
		} else {
			if (wc == 1) {
				uint32 wp;
				/* XXX handle LONG->SHORT conversion */
				TIFFGetField(tif, fip->field_tag, &wp);
				TDIRSetEntryOff(tif,dir, wp);
			} else {
/* ----------------------------------------------- */
/* this is the code that is called when I write my file */
				uint32* lp;
				TIFFGetField(tif, fip->field_tag, &lp);
				if (!TIFFWriteLongArray(tif, dir, lp))
					return 0;
			}
		}
		break;
[/code]

So, an uninitialized pointer lp is declared and its address passed to
TIFFGetField.  This in turn sets up the va_list and calls TIFFVGetField,
which calls _TIFFVGetField with the supplied va_list and the pointer to the
unitialized pointer.  

There are two problems here.

1. This is how the library extracts the data (my code, but again, following
the pattern already present)

*va_arg(ap, int32*) = td->td_xxx[0];

This seems incorrect.  It is setting the original pointer to the value of an
int.  I reasoned that perhaps, in the example I was following
(TIFFTAG_YCBCRSUBSAMPLING), these integers were actually addresses.  even if
that's the case though there is another problem.

The next problem is that we call va_args N times, where N is the number of
elements in the array. From what I see the variable argument list only ever
contains a single argument (the address of a pointer).  This is undefined
behavior per the standard (important bit at the begining):

[b]If there is no actual next argument[/b], or if type is not compatible
with the type of
the actual next argument (as promoted according to the default
argument promotions), the behavior is undefined.

The correct version would be 

*va_arg(ap, int32**) = td_xxx;

This sets the previously uninitialized pointer to the array, which is valid.

I don't like that it points to the data itself instead of a copy, but
whatever, at least it doesn't crash and gives me the correct result.

My concern here is that I am missing something subtle.  This software is old
and used by many, many people. As such, calling this a bug feels to me like
blaming a crash on the compiler, which is *almost* always wrong.  However, I
cannot reason out a way in which this is correct, specifically how the
library writes to what va_arg returns when called more than one time.

Any help would be greatly appreciated.  Thanks in advance.
-- 
View this message in context:
http://old.nabble.com/Confusion-in-implementing-_TIFFVGetField%2C-adding-a-custom-TAG-tp33409332p33409332.html
Sent from the Tiff / LibTiff mailing list archive at Nabble.com.