Skip to content

Commit

Permalink
Fixed various type punning warnings by using memcpy()
Browse files Browse the repository at this point in the history 
2acc444 was buggy, and undone with cedc42d.  This essentially restores 2acc444 without the buggy pointer arithmetic.

Modern compilers optimize memcpy() very well, this will likely produce the same code.
  • Loading branch information
@seanm
seanm committed Sep 11, 2023
1 parent 75180f7 commit 0011816
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 20 deletions.
46 changes: 35 additions & 11 deletions nifti2/nifti_tool.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,7 @@ static int g_debug = 1;
#include <limits.h>
#include <string.h>
#include <stdint.h>
#include <assert.h> // TEMP

#include "nifti2_io.h"
#include "nifti_tool.h"
Expand Down Expand Up @@ -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;

Expand All @@ -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 )
Expand All @@ -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);
Expand All @@ -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 )
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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;
}
}
Expand All @@ -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);
Expand Down
34 changes: 25 additions & 9 deletions niftilib/nifti1_tool.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,7 @@ static int g_debug = 1;
#include <limits.h>
#include <string.h>
#include <stdint.h>
#include <assert.h> // TEMP

#include "nifti1_io.h"
#include "nifti1_tool.h"
Expand Down Expand Up @@ -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;

Expand All @@ -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 )
Expand All @@ -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);
Expand All @@ -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 )
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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;
}
}
Expand Down

0 comments on commit 0011816

Please sign in to comment.