Skip to content

Commit e59d2c6

Browse files
malaterremalaterre
committed
Merge branch 'release'
2 parents 391e031 + 4172c18 commit e59d2c6

File tree

6 files changed

+73
-20
lines changed

6 files changed

+73
-20
lines changed

Diff for: ‎Applications/Cxx/gdcmclean.cxx

+8-1
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,10 @@ static void PrintHelp() {
138138
std::cout << " --preserve-illegal Whether or "
139139
"not preserve illegal attributes (eg. group 0003...)."
140140
<< std::endl;
141+
std::cout
142+
<< " --empty-when-scrub-fails Fallback to "
143+
"emptying any attribute for which scrub operation failed."
144+
<< std::endl;
141145
std::cout << "General Options:" << std::endl;
142146
std::cout << " -V --verbose more verbose (warning+error)."
143147
<< std::endl;
@@ -171,6 +175,7 @@ int main(int argc, char *argv[]) {
171175
int preserveAllMissingPrivateCreator = 0;
172176
int preserveAllGroupLength = 0;
173177
int preserveAllIllegal = 0;
178+
int emptyWhenScrubFails = 0;
174179
std::vector<gdcm::DPath> empty_dpaths;
175180
std::vector<gdcm::DPath> remove_dpaths;
176181
std::vector<gdcm::DPath> scrub_dpaths;
@@ -196,14 +201,15 @@ int main(int argc, char *argv[]) {
196201
{"recursive", no_argument, nullptr, 'r'},
197202
{"empty", required_argument, &empty_tag, 1}, // 3
198203
{"remove", required_argument, &remove_tag, 1}, // 4
199-
{"scrub", required_argument, &scrub_tag, 1}, // 5
204+
{"scrub", required_argument, &scrub_tag, 1}, // 5
200205
{"preserve", required_argument, &preserve_tag, 1}, // 5
201206
{"continue", no_argument, nullptr, 'c'},
202207
{"skip-meta", 0, &skipmeta, 1}, // should I document this one ?
203208
{"preserve-missing-private-creator", 0,
204209
&preserveAllMissingPrivateCreator, 1}, //
205210
{"preserve-group-length", 0, &preserveAllGroupLength, 1}, //
206211
{"preserve-illegal", 0, &preserveAllIllegal, 1}, //
212+
{"empty-when-scrub-fails", 0, &emptyWhenScrubFails, 1}, //
207213

208214
{"verbose", no_argument, nullptr, 'V'},
209215
{"warning", no_argument, nullptr, 'W'},
@@ -437,6 +443,7 @@ int main(int argc, char *argv[]) {
437443
cleaner.RemoveAllMissingPrivateCreator(!preserveAllMissingPrivateCreator);
438444
cleaner.RemoveAllGroupLength(!preserveAllGroupLength);
439445
cleaner.RemoveAllIllegal(!preserveAllIllegal);
446+
cleaner.EmptyWhenScrubFails(!!emptyWhenScrubFails);
440447
// Preserve
441448
for (std::vector<gdcm::DPath>::const_iterator it = preserve_dpaths.begin();
442449
it != preserve_dpaths.end(); ++it) {

Diff for: ‎Applications/Cxx/gdcmdump.cxx

+9-6
Original file line numberDiff line numberDiff line change
@@ -1137,8 +1137,9 @@ static int PrintCSA(const std::string & filename)
11371137
int ret = 0;
11381138
if( ds.FindDataElement( t1 ) )
11391139
{
1140-
csa.LoadFromDataElement( ds.GetDataElement( t1 ) );
1141-
csa.Print( std::cout );
1140+
if( csa.LoadFromDataElement( ds.GetDataElement( t1 ) ) )
1141+
csa.Print( std::cout );
1142+
else ret = 1;
11421143
found = true;
11431144
if( csa.GetFormat() == gdcm::CSAHeader::ZEROED_OUT )
11441145
{
@@ -1156,8 +1157,9 @@ static int PrintCSA(const std::string & filename)
11561157
}
11571158
if( ds.FindDataElement( t2 ) )
11581159
{
1159-
csa.LoadFromDataElement( ds.GetDataElement( t2 ) );
1160-
csa.Print( std::cout );
1160+
if( csa.LoadFromDataElement( ds.GetDataElement( t2 ) ) )
1161+
csa.Print( std::cout );
1162+
else ret = 1;
11611163
found = true;
11621164
if( csa.GetFormat() == gdcm::CSAHeader::ZEROED_OUT )
11631165
{
@@ -1175,8 +1177,9 @@ static int PrintCSA(const std::string & filename)
11751177
}
11761178
if( ds.FindDataElement( t3 ) )
11771179
{
1178-
csa.LoadFromDataElement( ds.GetDataElement( t3 ) );
1179-
csa.Print( std::cout );
1180+
if( csa.LoadFromDataElement( ds.GetDataElement( t3 ) ) )
1181+
csa.Print( std::cout );
1182+
else ret = 1;
11801183
found = true;
11811184
if( csa.GetFormat() == gdcm::CSAHeader::ZEROED_OUT )
11821185
{

Diff for: ‎Source/DataStructureAndEncodingDefinition/gdcmCSAHeader.cxx

+9-1
Original file line numberDiff line numberDiff line change
@@ -910,7 +910,8 @@ bool check_mapping(uint32_t syngodt, const char *vr)
910910
{
911911
static const unsigned int max = sizeof(mapping) / sizeof(equ);
912912
const equ *p = mapping;
913-
assert( syngodt <= mapping[max-1].syngodt ); (void)max;
913+
if( syngodt > mapping[max-1].syngodt ) return false;
914+
assert( syngodt <= mapping[max-1].syngodt );
914915
while(p->syngodt < syngodt )
915916
{
916917
//std::cout << "mapping:" << p->vr << std::endl;
@@ -1098,6 +1099,11 @@ bool CSAHeader::LoadFromDataElement(DataElement const &de)
10981099
char vr[4];
10991100
ss.read(vr, 4);
11001101
// In dataset without magic signature (OLD FORMAT) vr[3] is garbage...
1102+
if( vr[2] != 0 )
1103+
{
1104+
gdcmErrorMacro( "Garbage data. Stopping CSA parsing." );
1105+
return false;
1106+
}
11011107
assert( /*vr[3] == 0 &&*/ vr[2] == 0 );
11021108
csael.SetVR( VR::GetVRTypeFromFile(vr) );
11031109
//std::cout << "VR " << vr << ", ";
@@ -1131,8 +1137,10 @@ bool CSAHeader::LoadFromDataElement(DataElement const &de)
11311137
uint32_t item_xx[4];
11321138
ss.read((char*)&item_xx, 4*sizeof(uint32_t));
11331139
SwapperNoOp::SwapArray(item_xx,4);
1140+
if( item_xx[2] != 77 && item_xx[2] != 205 ) return false;
11341141
assert( item_xx[2] == 77 || item_xx[2] == 205 );
11351142
uint32_t len = item_xx[1]; // 2nd element
1143+
if( item_xx[0] != item_xx[1] || item_xx[1] != item_xx[3] ) return false;
11361144
assert( item_xx[0] == item_xx[1] && item_xx[1] == item_xx[3] );
11371145
if( len )
11381146
{

Diff for: ‎Source/MediaStorageAndFileFormat/gdcmCleaner.cxx

+28-5
Original file line numberDiff line numberDiff line change
@@ -538,10 +538,12 @@ struct Cleaner::impl {
538538
bool AllMissingPrivateCreator;
539539
bool AllGroupLength;
540540
bool AllIllegal;
541+
bool WhenScrubFails;
541542
impl()
542543
: AllMissingPrivateCreator(true),
543544
AllGroupLength(true),
544-
AllIllegal(true) {}
545+
AllIllegal(true),
546+
WhenScrubFails(false) {}
545547

546548
enum ACTION { NONE, EMPTY, REMOVE, SCRUB };
547549
enum ACTION ComputeAction(File const &file, DataSet &ds,
@@ -668,6 +670,10 @@ struct Cleaner::impl {
668670
bool RemoveMissingPrivateCreator(Tag const & /*t*/) { return false; }
669671
void RemoveAllGroupLength(bool remove) { AllGroupLength = remove; }
670672
void RemoveAllIllegal(bool remove) { AllIllegal = remove; }
673+
void EmptyWhenScrubFails(bool empty) { WhenScrubFails = empty; }
674+
675+
bool CleanCSAImage(DataSet &ds, const DataElement &de);
676+
bool CleanCSASeries(DataSet &ds, const DataElement &de);
671677
};
672678

673679
static VR ComputeDictVR(File &file, DataSet &ds, DataElement const &de) {
@@ -840,7 +846,7 @@ static inline bool bs_is_signature(const ByteValue *bv, const char *str) {
840846
return false;
841847
}
842848

843-
static bool CleanCSAImage(DataSet &ds, const DataElement &de) {
849+
bool Cleaner::impl::CleanCSAImage(DataSet &ds, const DataElement &de) {
844850
const ByteValue *bv = de.GetByteValue();
845851
// fast path:
846852
if (!bv) return true;
@@ -888,6 +894,13 @@ static bool CleanCSAImage(DataSet &ds, const DataElement &de) {
888894
gdcmDebugMacro("Zero-out CSA header");
889895
return true;
890896
}
897+
// fallback logic:
898+
if (WhenScrubFails && is_signature(bv, sv10)) {
899+
// so SV10 header has been identified, but we failed to 'scrub', let's
900+
// empty it:
901+
ds.Replace(clean);
902+
return true;
903+
}
891904
gdcmErrorMacro("Failure to call CleanCSAImage");
892905
return false;
893906
}
@@ -900,7 +913,7 @@ static bool CleanCSAImage(DataSet &ds, const DataElement &de) {
900913
return true;
901914
}
902915

903-
static bool CleanCSASeries(DataSet &ds, const DataElement &de) {
916+
bool Cleaner::impl::CleanCSASeries(DataSet &ds, const DataElement &de) {
904917
const ByteValue *bv = de.GetByteValue();
905918
// fast path:
906919
if (!bv) return true;
@@ -944,6 +957,13 @@ static bool CleanCSASeries(DataSet &ds, const DataElement &de) {
944957
gdcmDebugMacro("Zero-out CSA header");
945958
return true;
946959
}
960+
// fallback logic:
961+
if (WhenScrubFails && is_signature(bv, sv10)) {
962+
// so SV10 header has been identified, but we failed to 'scrub', let's
963+
// empty it:
964+
ds.Replace(clean);
965+
return true;
966+
}
947967
gdcmErrorMacro("Failure to call CleanCSASeries");
948968
return false;
949969
}
@@ -1065,8 +1085,8 @@ static bool IsDPathInSet(std::set<DPath> const &aset, DPath const dpath) {
10651085
}
10661086

10671087
Cleaner::impl::ACTION Cleaner::impl::ComputeAction(
1068-
File const & /*file*/, DataSet &ds, const DataElement &de, VR const &ref_dict_vr,
1069-
const std::string &tag_path) {
1088+
File const & /*file*/, DataSet &ds, const DataElement &de,
1089+
VR const &ref_dict_vr, const std::string &tag_path) {
10701090
const Tag &tag = de.GetTag();
10711091
// Group Length & Illegal cannot be preserved so it is safe to do them now:
10721092
if (tag.IsGroupLength()) {
@@ -1302,6 +1322,9 @@ void Cleaner::RemoveAllGroupLength(bool remove) {
13021322
pimpl->RemoveAllGroupLength(remove);
13031323
}
13041324
void Cleaner::RemoveAllIllegal(bool remove) { pimpl->RemoveAllIllegal(remove); }
1325+
void Cleaner::EmptyWhenScrubFails(bool empty) {
1326+
pimpl->EmptyWhenScrubFails(empty);
1327+
}
13051328

13061329
bool Cleaner::Clean() {
13071330
DataSet &ds = F->GetDataSet();

Diff for: ‎Source/MediaStorageAndFileFormat/gdcmCleaner.h

+3
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,9 @@ class GDCM_EXPORT Cleaner : public Subject {
6565
/// Should I remove all illegal attribute. Default: true
6666
void RemoveAllIllegal(bool remove);
6767

68+
/// Should I empty instead of scrub upon failure
69+
void EmptyWhenScrubFails(bool empty);
70+
6871
/// main loop
6972
bool Clean();
7073

Diff for: ‎Utilities/gdcmext/csa.c

+16-7
Original file line numberDiff line numberDiff line change
@@ -89,8 +89,15 @@ static void setup_buffer(struct app *self, void *output, const void *input,
8989
self->out->write = stream_write;
9090
}
9191

92+
//#define CSA_DEBUG
93+
94+
#ifdef CSA_DEBUG
95+
#define ERROR_RETURN(X, Y) \
96+
if ((X) != (Y)) assert(0);
97+
#else
9298
#define ERROR_RETURN(X, Y) \
9399
if ((X) != (Y)) return false
100+
#endif
94101

95102
static size_t fread_mirror(void *ptr, size_t size, size_t nmemb,
96103
struct app *self) {
@@ -214,16 +221,18 @@ static bool read_info(struct app *self, struct csa_info *i) {
214221
s = fread_mirror(&i->vm, sizeof i->vm, 1, self);
215222
ERROR_RETURN(s, 1);
216223
// vm == 115301884 seems to be ok...
217-
assert(i->vm < 0x6df5dfd);
224+
ERROR_RETURN(i->vm < 0x6df5dfd, true);
218225
// vr
219226
s = fread_mirror(i->vr, sizeof *i->vr, sizeof i->vr / sizeof *i->vr, self);
220227
ERROR_RETURN(s, 4);
221228
{
222229
const char *s = i->vr;
223-
assert(s[0] >= 'A' && s[0] <= 'Z');
224-
assert(s[1] >= 'A' && s[1] <= 'Z');
225-
assert(s[2] == 0);
226-
if (self->csa_type == SV10) assert(s[3] == 0);
230+
ERROR_RETURN(s[0] >= 'A' && s[0] <= 'Z', true);
231+
ERROR_RETURN(s[1] >= 'A' && s[1] <= 'Z', true);
232+
ERROR_RETURN(s[2], 0);
233+
if (self->csa_type == SV10) {
234+
ERROR_RETURN(s[3], 0);
235+
}
227236
}
228237
// syngodt (signed)
229238
s = fread_mirror(&i->syngodt, sizeof i->syngodt, 1, self);
@@ -250,15 +259,15 @@ static bool read_data(struct app *self, struct csa_item_data *d) {
250259
uint32_t unused;
251260
s = fread_mirror(&unused, sizeof unused, 1, self);
252261
ERROR_RETURN(s, 1);
253-
assert(unused == d->len || unused == 0x4d || unused == 0xcd);
262+
ERROR_RETURN(unused == d->len || unused == 0x4d || unused == 0xcd, true);
254263
}
255264

256265
if (d->len != 0) {
257266
const uint32_t padding_len = (4 - d->len % 4) % 4;
258267
const uint32_t padded_len = ((d->len + 3u) / 4u) * 4u; // (len + 3) & ~0x03
259268
assert(padded_len == d->len + padding_len); // programmer error
260-
d->buffer = (char *)realloc(d->buffer, padded_len);
261269
assert(padded_len != 0);
270+
d->buffer = (char *)realloc(d->buffer, padded_len);
262271
ERROR_RETURN(d->buffer != NULL, true);
263272
s = fread_mirror(d->buffer, sizeof *d->buffer, padded_len, self);
264273
ERROR_RETURN(s, padded_len);

0 commit comments

Comments
 (0)