Enet4 / dicom-rs

Rust implementation of the DICOM standard
https://dicom-rs.github.io
Apache License 2.0
416 stars 81 forks source link

InMemDicomObject Length::UNDEFINED after modify #364

Closed jmlaka closed 1 year ago

jmlaka commented 1 year ago

As an InMemDicomObject can have a certain defined Length after parsing dicom data, it is necessary to reset or recalculate this Length after calling any methods that modify the object's inner entries value e.g. .put() .remove_element()...

The Length is set to Length::UNDEFINED as recalculating it might be expensive.

jmlaka commented 1 year ago

Right, sorry, did not notice.

po 12. 6. 2023 o 10:54 Eduardo Pinho @.***> napísal(a):

@.**** commented on this pull request.

In object/src/mem.rs https://github.com/Enet4/dicom-rs/pull/364#discussion_r1226315507:

@@ -728,8 +745,8 @@ where let vr = e.vr(); // replace element *e = DataElement::empty(tag, vr);

  • self.len = Length::UNDEFINED;

If I see this correctly, actions SetStr, Set, and the various Push might also modify data without calling put (see e.g. line 811), so we'd also need some length resets in applypush_impl and apply_change_value_impl.

— Reply to this email directly, view it on GitHub https://github.com/Enet4/dicom-rs/pull/364#pullrequestreview-1474393499, or unsubscribe https://github.com/notifications/unsubscribe-auth/ARWGC3H24Z4TWIM5VOB3DDDXK3KMJANCNFSM6AAAAAAZCJZLGE . You are receiving this because you authored the thread.Message ID: @.***>

-- Juraj Mláka

jmlaka commented 1 year ago

I just removed some duplicate code.

The methods you mentioned did not need changes, as they internally call self.put() - which is already handled.

jmlaka commented 1 year ago

Still wrong :) Will be back