[fitsbits] Fwd: Possible bug in CFITSIO

[William Pence]




The HEASARC at NASA/GSFC continues to maintain the CFITSIO library.  Any issues with CFITSIO should be addressed to their help desk at ccfits at heasarc.gsfc.nasa.gov.  I’m forwarding this email to them for possible follow up.  

While the topic of how CFITSIO internally handles datatype conversion between every possible FITS datatype and every possible C variable datatype is too complex to go into here, the short answer is that it is being handled correctly.  This particular code has been widely used for the past 25 years, so any serious bugs would surely have been discovered by now. 

Bill Pence
HEASRC, NASA/GSFC, Emeritus 
——————————————-


Begin forwarded message:

> From: "David C. Partridge via fitsbits" <fitsbits at listmgr.nrao.edu>
> Date: February 27, 2020 at 6:56:03 AM EST
> To: fitsbits at nrao.edu
> Subject: [fitsbits] Possible bug in CFITSIO
> Reply-To: "David C. Partridge" <david.partridge at perdrix.co.uk>
> 
> ===== QUOTE =====
> Although FITS does not directly support unsigned integers as one of its
> fundamental data types, FITS can still be used to efficiently store unsigned
> integer data values in images and binary tables. The convention used in FITS
> files is to store the unsigned integers as signed integers with an
> associated offset (specified by the BZERO or TZEROn keyword). For example,
> to store unsigned 16-bit integer values in a FITS image the image would be
> defined as a signed 16-bit integer (with BITPIX keyword = SHORT_IMG = 16)
> with the keywords BSCALE = 1.0 and BZERO = 32768. Thus the unsigned values
> of 0, 32768, and 65535, for example, are physically stored in the FITS image
> as -32768, 0, and 32767, respectively; CFITSIO automatically adds the BZERO
> offset to these values when they are read.
> ===== END QUOTE =====
> 
> With that in mind I believe that the code in fffi2i2 (for example) at line
> 1054 is incorrect as it always adds the zero value before checking for an
> overflow error.
> 
> I think that it should check the value is in range and ONLY then add the
> zero value.
> 
> IOW:
> 
>            for (ii = 0; ii < ntodo; ii++)
>            {
>                dvalue = input[ii] * scale;   // don't do + zero;
> 
>                if (dvalue < DSHRT_MIN)
>                {
>                    *status = OVERFLOW_ERR;
>                    dvalue = DSHRT_MIN;
>                }
>                else if (dvalue > DSHRT_MAX)
>                {
>                    *status = OVERFLOW_ERR;
>                    dvalue = DSHRT_MAX;
>                };
>                dvalue += zero;    // NOW Add the zero offset to the validated
> value
>                output[ii] = (short) dvalue;
>            }
> 
> Similar issue with handling of 32 bit values.
> 
> David
> 
> 
> _______________________________________________
> fitsbits mailing list
> fitsbits at listmgr.nrao.edu
> https://listmgr.nrao.edu/mailman/listinfo/fitsbits
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listmgr.nrao.edu/pipermail/fitsbits/attachments/20200227/fc758b3c/attachment-0002.html>