Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TIFF/tif_print.c:282: suspicious for loop ? #35

Open
dcb314 opened this issue Aug 28, 2020 · 4 comments
Open

TIFF/tif_print.c:282: suspicious for loop ? #35

dcb314 opened this issue Aug 28, 2020 · 4 comments

Comments

@dcb314
Copy link

dcb314 commented Aug 28, 2020

TIFF/tif_print.c:282:30: warning: variable 'i' used in loop condition not modified in loop body [-Wfor-loop-analysis]

Source code is

           for (cp = td->td_inknames; i > 0; cp = strchr(cp, '\0')) {
                    fprintf(fd, "%s%s", sep, cp);
                    sep = ", ";
            }
@nrnhines
Copy link
Member

Thanks. Clearly an error. Dates back to the original source. But it is not clear to me what the proper fix is. Is cp a pointer to a list of chars that contains possibly many '\0'? But cp would never increment past the first.

@dcb314
Copy link
Author

dcb314 commented May 1, 2024

Still broken nearly four years later.

@nrnhines
Copy link
Member

nrnhines commented May 1, 2024

I know nothing, (next to nothing?), about TIFF.

Searching for libtiff.c I found many versions, some with the above bug. One that is very similar to the above but with a fix is
https://git.acem.ece.illinois.edu/lib/VTK-5.0.0/src/commit/1a1c91536bf442bb3d17be57dfdb6a8bd37838cc/Utilities/vtktiff/tif_print.c starting at line 268

        if (TIFFFieldSet(tif,FIELD_INKNAMES)) {
                char* cp;
                fprintf(fd, "  Ink Names: ");
                i = td->td_samplesperpixel;
                sep = "";
                for (cp = td->td_inknames; i > 0; cp = strchr(cp,'\0')+1, i--) {
                        fprintf(fd, "%s", sep);
                        _TIFFprintAscii(fd, cp);
                        sep = ", ";
                }
        }

and another at https://android.googlesource.com/platform/external/free-image/+/71163caf89862f79f6419077cf6f0b18b7725d52/Source/LibTIFF/tif_print.c starting at line 342 that is also very similar and an extra line at the end

		char* cp;
		fprintf(fd, "  Ink Names: ");
		i = td->td_samplesperpixel;
		sep = "";
		for (cp = td->td_inknames; i > 0; cp = strchr(cp,'\0')+1, i--) {
			fputs(sep, fd);
			_TIFFprintAscii(fd, cp);
			sep = ", ";
		}
                fputs("\n", fd);
	}

Github has a very large number of libtiff repositories. One that struck me as undergoing active development is
https://gitlab.com/libtiff/libtiff and
https://gitlab.com/libtiff/libtiff/-/blob/master/libtiff/tif_print.c has the fragment at line 404 (at least a family resemblance in code)

    if (TIFFFieldSet(tif, FIELD_INKNAMES))
    {
        char *cp;
        uint16_t i;
        fprintf(fd, "  Ink Names: ");
        i = td->td_samplesperpixel;
        sep = "";
        for (cp = td->td_inknames;
             i > 0 && cp < td->td_inknames + td->td_inknameslen;
             cp = strchr(cp, '\0') + 1, i--)
        {
            size_t max_chars = td->td_inknameslen - (cp - td->td_inknames);
            fputs(sep, fd);
            _TIFFprintAsciiBounded(fd, cp, max_chars);
            sep = ", ";
        }
        fputs("\n", fd);
    }

I wonder if the simplest fix above fixes your issue?

@dcb314
Copy link
Author

dcb314 commented May 1, 2024

I wonder if the simplest fix above fixes your issue?

I am wondering why this project wants to copy out of date source code.

Perhaps linking to the latest version would be a better idea ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants