From 0011816192f7d5460e5cebe9093e331732068492 Mon Sep 17 00:00:00 2001 From: Sean McBride Date: Sat, 1 Apr 2023 17:16:00 -0400 Subject: [PATCH] Fixed various type punning warnings by using memcpy() 2acc4449 was buggy, and undone with cedc42d2. This essentially restores 2acc4449 without the buggy pointer arithmetic. Modern compilers optimize memcpy() very well, this will likely produce the same code. --- nifti2/nifti_tool.c | 46 ++++++++++++++++++++++++++++++++---------- niftilib/nifti1_tool.c | 34 ++++++++++++++++++++++--------- 2 files changed, 60 insertions(+), 20 deletions(-) diff --git a/nifti2/nifti_tool.c b/nifti2/nifti_tool.c index c880c8e..46ee404 100644 --- a/nifti2/nifti_tool.c +++ b/nifti2/nifti_tool.c @@ -211,6 +211,7 @@ static int g_debug = 1; #include #include #include +#include // TEMP #include "nifti2_io.h" #include "nifti_tool.h" @@ -3828,7 +3829,6 @@ int modify_all_fields( void * basep, nt_opts * opts, field_s * fields, int flen) *----------------------------------------------------------------------*/ int modify_field(void * basep, field_s * field, const char * data) { - float fval; const char * posn = data; int val, max, fc, nchars; @@ -3854,7 +3854,7 @@ int modify_field(void * basep, field_s * field, const char * data) case DT_INT8: { - max = 127; + max = INT8_MAX; for( fc = 0; fc < field->len; fc++ ) { if( sscanf(posn, " %d%n", &val, &nchars) != 1 ) @@ -3871,7 +3871,8 @@ int modify_field(void * basep, field_s * field, const char * data) return 1; } /* otherwise, we're good */ - (((char *)basep + field->offset))[fc] = (char)val; + char * offsetp = (char *)basep + field->offset; + offsetp[fc] = (char)val; if( g_debug > 1 ) fprintf(stderr,"+d setting posn %d of '%s' to %d\n", fc, field->name, val); @@ -3882,7 +3883,7 @@ int modify_field(void * basep, field_s * field, const char * data) case DT_INT16: { - max = 32767; + max = INT16_MAX; for( fc = 0; fc < field->len; fc++ ) { if( sscanf(posn, " %d%n", &val, &nchars) != 1 ) @@ -3899,7 +3900,12 @@ int modify_field(void * basep, field_s * field, const char * data) return 1; } /* otherwise, we're good */ - ((short *)((char *)basep + field->offset))[fc] = (short)val; + int16_t sval = (int16_t)val; + char * offsetp = (char *)basep + field->offset; + offsetp = &(offsetp[fc * sizeof(sval)]); + assert(offsetp == &(((short *)((char *)basep + field->offset))[fc])); // TEMP + assert(0); + memcpy(offsetp, &sval, sizeof(sval)); if( g_debug > 1 ) fprintf(stderr,"+d setting posn %d of '%s' to %d\n", fc, field->name, val); @@ -3918,7 +3924,12 @@ int modify_field(void * basep, field_s * field, const char * data) fc,field->len); return 1; } - ((int *)((char *)basep + field->offset))[fc] = val; + int32_t ival = (int32_t)val; + char * offsetp = (char *)basep + field->offset; + offsetp = &(offsetp[fc * sizeof(ival)]); + assert(offsetp == &(((int *)((char *)basep + field->offset))[fc])); // TEMP + assert(0); + memcpy(offsetp, &ival, sizeof(ival)); if( g_debug > 1 ) fprintf(stderr,"+d setting posn %d of '%s' to %d\n", fc, field->name, val); @@ -3938,7 +3949,11 @@ int modify_field(void * basep, field_s * field, const char * data) fc,field->len); return 1; } - ((int64_t *)((char *)basep + field->offset))[fc] = v64; + char * offsetp = (char *)basep + field->offset; + offsetp = &(offsetp[fc * sizeof(v64)]); + assert(offsetp == &(((int64_t *)((char *)basep + field->offset))[fc])); // TEMP + assert(0); + memcpy(offsetp, &v64, sizeof(v64)); if( g_debug > 1 ) fprintf(stderr,"+d setting posn %d of '%s' to %" PRId64 "\n", fc, field->name, v64); @@ -3949,19 +3964,24 @@ int modify_field(void * basep, field_s * field, const char * data) case DT_FLOAT32: { + float f32; for( fc = 0; fc < field->len; fc++ ) { - if( sscanf(posn, " %f%n", &fval, &nchars) != 1 ) + if( sscanf(posn, " %f%n", &f32, &nchars) != 1 ) { fprintf(stderr,"** found %d of %d modify values\n", fc,field->len); return 1; } /* otherwise, we're good */ - ((float *)((char *)basep + field->offset))[fc] = fval; + char * offsetp = (char *)basep + field->offset; + offsetp = &(offsetp[fc * sizeof(f32)]); + assert(offsetp == &(((float *)((char *)basep + field->offset))[fc])); // TEMP + assert(0); + memcpy(offsetp, &f32, sizeof(f32)); if( g_debug > 1 ) fprintf(stderr,"+d setting posn %d of '%s' to %f\n", - fc, field->name, fval); + fc, field->name, f32); posn += nchars; } } @@ -3979,7 +3999,11 @@ int modify_field(void * basep, field_s * field, const char * data) return 1; } /* otherwise, we're good */ - ((double *)((char *)basep + field->offset))[fc] = f64; + char * offsetp = (char *)basep + field->offset; + offsetp = &(offsetp[fc * sizeof(f64)]); + assert(offsetp == &(((double *)((char *)basep + field->offset))[fc])); // TEMP + assert(0); + memcpy(offsetp, &f64, sizeof(f64)); if( g_debug > 1 ) fprintf(stderr,"+d setting posn %d of '%s' to %f\n", fc, field->name, f64); diff --git a/niftilib/nifti1_tool.c b/niftilib/nifti1_tool.c index f5908cd..11c5afb 100644 --- a/niftilib/nifti1_tool.c +++ b/niftilib/nifti1_tool.c @@ -165,6 +165,7 @@ static int g_debug = 1; #include #include #include +#include // TEMP #include "nifti1_io.h" #include "nifti1_tool.h" @@ -2893,7 +2894,6 @@ int modify_all_fields( void * basep, nt_opts * opts, field_s * fields, int flen) *----------------------------------------------------------------------*/ int modify_field(void * basep, field_s * field, const char * data) { - float fval; const char * posn = data; int val, max, fc, nchars; @@ -2919,7 +2919,7 @@ int modify_field(void * basep, field_s * field, const char * data) case DT_INT8: { - max = 127; + max = INT8_MAX; for( fc = 0; fc < field->len; fc++ ) { if( sscanf(posn, " %d%n", &val, &nchars) != 1 ) @@ -2936,7 +2936,8 @@ int modify_field(void * basep, field_s * field, const char * data) return 1; } /* otherwise, we're good */ - (((char *)basep + field->offset))[fc] = (char)val; + char * offsetp = (char *)basep + field->offset; + offsetp[fc] = (char)val; if( g_debug > 1 ) fprintf(stderr,"+d setting posn %d of '%s' to %d\n", fc, field->name, val); @@ -2947,7 +2948,7 @@ int modify_field(void * basep, field_s * field, const char * data) case DT_INT16: { - max = 32767; + max = INT16_MAX; for( fc = 0; fc < field->len; fc++ ) { if( sscanf(posn, " %d%n", &val, &nchars) != 1 ) @@ -2964,7 +2965,12 @@ int modify_field(void * basep, field_s * field, const char * data) return 1; } /* otherwise, we're good */ - ((short *)((char *)basep + field->offset))[fc] = (short)val; + int16_t sval = (int16_t)val; + char * offsetp = (char *)basep + field->offset; + offsetp = &(offsetp[fc * sizeof(sval)]); + assert(offsetp == &(((short *)((char *)basep + field->offset))[fc])); // TEMP + assert(0); + memcpy(offsetp, &sval, sizeof(sval)); if( g_debug > 1 ) fprintf(stderr,"+d setting posn %d of '%s' to %d\n", fc, field->name, val); @@ -2983,7 +2989,12 @@ int modify_field(void * basep, field_s * field, const char * data) fc,field->len); return 1; } - ((int *)((char *)basep + field->offset))[fc] = val; + int32_t ival = (int32_t)val; + char * offsetp = (char *)basep + field->offset; + offsetp = &(offsetp[fc * sizeof(ival)]); + assert(offsetp == &(((int *)((char *)basep + field->offset))[fc])); // TEMP + assert(0); + memcpy(offsetp, &ival, sizeof(ival)); if( g_debug > 1 ) fprintf(stderr,"+d setting posn %d of '%s' to %d\n", fc, field->name, val); @@ -2994,19 +3005,24 @@ int modify_field(void * basep, field_s * field, const char * data) case DT_FLOAT32: { + float f32; for( fc = 0; fc < field->len; fc++ ) { - if( sscanf(posn, " %f%n", &fval, &nchars) != 1 ) + if( sscanf(posn, " %f%n", &f32, &nchars) != 1 ) { fprintf(stderr,"** found %d of %d modify values\n", fc,field->len); return 1; } /* otherwise, we're good */ - ((float *)((char *)basep + field->offset))[fc] = fval; + char * offsetp = (char *)basep + field->offset; + offsetp = &(offsetp[fc * sizeof(f32)]); + assert(offsetp == &(((float *)((char *)basep + field->offset))[fc])); // TEMP + assert(0); + memcpy(offsetp, &f32, sizeof(f32)); if( g_debug > 1 ) fprintf(stderr,"+d setting posn %d of '%s' to %f\n", - fc, field->name, fval); + fc, field->name, f32); posn += nchars; } }