2016.11.16 12:29 "Re: [Tiff] Global variables in LibTIFF", by Yakov Galka
On Wed, Nov 16, 2016 at 2:04 PM, Gerben Vos <Gerben.Vos@zylab.com> wrote:
Why is the context named? It smells like magic behavior at a distance (even though it doesn't seem to be) that the name of the context and the opened tiff are the same.
I don't understand your complaints. It is already in the libTIFF API that you can create a context with one name and use it to read a file of another name. E.g.:
TIFFClientOpen("junk",... functions which read from whatever ...);
TIFFFdOpen(open("a.txt"), "b.txt", "r);
It is just a feature of the existing API that the TIFF structure has an associated name, and this name is used mostly for error reporting.
The term "filename" used currently by the library is actually misleading, because as with the case of TIFFClientOpen it might correspond to something that's not really an FS file.
It is also a feature of the existing implementation that this name is allocated as part of the TIFF structure (in one malloc call), so it has to be passed as part of TIFF creation rather than set later by a different API call.
I did a thread-safety analysis a few years ago, but did not contact the
mailing list about it. My implementation ideas for fixing those were more among the lines of those in the patch attached to http://bugzilla.maptools.org/show_bug.cgi?id=2255, except that I thought of calling the context object with the formerly global variables a TIFFContext instead of a TIFFApplication. There would then be a default context (that would still be the, only, global variable) that would be used by code that didn’t create their own context. I don't have an implementation, though.
Here are the main notes from my thread-safety analysis at that time (September 2014; I think it was for libtiff 4.0.3):
- TIFFSetErrorHandler/TIFFSetErrorHandlerExt/TIFFError/TIFFErrorExt: TIFFErrorHandlerExt _TIFFerrorHandlerExt; TIFFErrorHandler _TIFFerrorHandler;
- TIFFSetWarningHandler/TIFFSetWarningHandlerExt/TIFFWarning/TIFFWarningExt: TIFFErrorHandlerExt _TIFFwarningHandlerExt; TIFFErrorHandler _TIFFwarningHandler;
- TIFFSetTagExtender/TIFFDefaultDirectory: static TIFFExtendProc _TIFFextender;
- TIFFRegisterCODEC/TIFFUnRegisterCODEC: static codec_t* registeredCODECS; (linked list)
- TIFFFindCODEC/TIFFIsCODECConfigured/TIFFGetConfiguredCODECs: static codec_t* registeredCODECS; (linked list)
- TIFFPrintDirectory: static uint16 dotrange;
- oog_encode: static int oog_table[NANGLES]; static int initialized = 0;
- PixarLogMakeTables/PixarLogEncode: static float Fltsize; static float LogK1, LogK2;
I would add a future feature request: custom memory allocator.
Even though it is more logical to put CODECs, extenders, etc into a separate context that is shared by multiple TIFF structures, I believe that TIFF-specific callbacks should be part of the TIFF, or receive the TIFF* that caused the callback as one of their arguments.
My patch addresses only the Error/Warning part of these because it is the most important part for me at this point in time. (Currently we have an ugly TLS workaround here, which causes me mental pain every time I accidentally see it, especially in contrast with other libraries, like openjpeg, which handle it properly.)