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

no warning if I/O buffer too small in pipe #17

Open
teuben opened this issue Jun 16, 2019 · 9 comments
Open

no warning if I/O buffer too small in pipe #17

teuben opened this issue Jun 16, 2019 · 9 comments

Comments

@teuben
Copy link
Owner

teuben commented Jun 16, 2019

In this example:
fitsccd tmp1.fits - | ccdstat -
for small files this works, as long as the file fits in memory, but there is no warning if not. then ccdstat will just report a 0 data array. Not good.

This should not happen, as filestruct allocated all data in memory when coming from pipe.

@teuben
Copy link
Owner Author

teuben commented Aug 20, 2023

  ccdgen cube1 flat 1 1500,1500,1500; ccdfits cube1 cube1.fits
  ### Fatal error [ccdfits]: [image.c:323]: cannot allocate 18446744066349813248 bytes

this also fails, but it seem fitsio cannot handle files > 4GB

@teuben
Copy link
Owner Author

teuben commented Jan 2, 2025

Converted the filestruct code to using off_t and size_t an odd (compiler?) bug came up where the following seemingly legal code failed to stop the while loop:

        size_t   len;

	while (--len >= 0) {			/*   loop copying data      */
	    *dat++ = *src++;			/*     byte by byte         */
	}

whereas the following code ran fine:

	for (size_t i=0; i<len; i++)
	  *dat++ = *src++;

maybe I need another eye on this @jcldc ??? Happy newyear JC !

Code is still in the "issue17" branch, not merged to master yet.

@teuben
Copy link
Owner Author

teuben commented Jan 3, 2025

All of the examples given above now seem to work, except scanfits on huge fits files fails. fitshead works fine though. This must be an unrelated and scanfits specific error due to some never converted 32/64 bit code.

@teuben
Copy link
Owner Author

teuben commented Jan 4, 2025

scanfits and the fits library module also needed a patch for using size_t instead of int. All example now workingl.

@jcldc
Copy link
Collaborator

jcldc commented Jan 6, 2025

Hi Peter,
an unsigned int variable cannot have a negative value. It leads to an underflow, which corrupt the memory, and involve an infinite while-loop and then crash. This is why while (--len >= 0) statement does not work.
Here is a simple example which reproduce the bug.

#include <stdio.h>
#define MAXLEN 10
int main()
{
    size_t len;
    int bsrc[MAXLEN], bdat[MAXLEN],i;
    int * src, * dat;
    
    len = MAXLEN;
    for (i=0; i<len; i++) {
        bsrc[i] = i;
    }
    src = bsrc;
    dat = bdat;
    printf("First len = %ld\n\n",len);
#if 1    
    while (--len >= 0) {
        printf("-> len=%lu",len);
        *dat++ = *src++;
    }
#else
    for (size_t i=0; i<len; i++)
	    *dat++ = *src++;
#endif    
}

@jcldc
Copy link
Collaborator

jcldc commented Jan 6, 2025

Actually, if arithmetic underflow occurs, such as when decrementing past zero, all bits in size_t flip to one, representing the maximum unsigned value, not a negative number.

@jcldc
Copy link
Collaborator

jcldc commented Jan 6, 2025

Actually, if arithmetic underflow occurs, such as when decrementing past zero, all bits in size_t flip to one, representing the maximum unsigned value, not a negative number.

I have updated the example code above, to display the real unsigned int value of len :
printf("-> len=%lu",len);

@teuben
Copy link
Owner Author

teuben commented Jan 6, 2025

ah, indeed, i need to change the %ld to %lu in the code! In Issue 17 was merged already, so I''ll do that in the master branch.

@teuben
Copy link
Owner Author

teuben commented Jan 8, 2025

I benchmarked the while() and for() loop, using the intel timers(3NEMO) routines. For small values of the array (say < 1e4) for() is just a tad slower than while(), but for much larger values of the array, the while() loop can be 2-3 times faster.

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

No branches or pull requests

2 participants