[fitsbits] Possible bug in CFITSIO

[David C. Partridge]

So for clarity:

 

Are you saying that if the headers of a 16 bit FITS file say:

 

    SIMPLE  =                    T / file does conform to FITS standard

    BITPIX  =                   16 / number of bits per data pixel

    BZERO   =                32768 / offset data range to that of unsigned short

    BSCALE  =                    1 / default scaling factor

 

that I should ignore the fact the BITPIX is telling me to use signed short, and act as if

BITPIX had a value of 20 (unsigned short).

 

And similarly 32 bit files with values of BITPX of 32 (signed) and 40 (unsigned)??

 

That somehow feels wrong!

 

David

 

From: William Pence [mailto:wdpence2000 at yahoo.com] 
Sent: 27 February 2020 16:56
To: CCFITS Help Desk
Cc: david.partridge at perdrix.co.uk; fitsbits at listmgr.nrao.edu
Subject: Fwd: [fitsbits] Possible bug in CFITSIO

 







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/20200228/bf683db6/attachment.html>