Slightly changed implementation of DcmVR::setVR().

Slightly changed implementation of DcmVR::setVR(): now, internal VRs are
not treated differently, but they are either mapped to an unknown VR
with extended length (4 bytes) or with a 2-byte length field, depending
on the characters used for the VR string.

Also enhanced the documentation of this method and made a few local
variables "const".

dcmdata/include/dcmtk/dcmdata/dcvr.h

@@ -305,7 +305,7 @@ class DCMTK_DCMDATA_EXPORT DcmVR
     /** constructor.
      *  Please note that only the first two characters of the passed string are
      *  actually checked.  Value Representations that are labeled for internal
-     *  use only are mapped to EVR_UNKNOWN.
+     *  use only are mapped to an unknown VR.  See setVR() for more details.
      *  @param vrName symbolic name of value representation
      */
     DcmVR(const char* vrName)
@@ -330,7 +330,13 @@ class DCMTK_DCMDATA_EXPORT DcmVR
     /** assign new VR value by name.
      *  Please note that only the first two characters of the passed string are
      *  actually checked.  Value Representations that are labeled for internal
-     *  use only are mapped to EVR_UNKNOWN.
+     *  use only are ignored, so they are mapped to an unknown VR.  For unknown
+     *  VRs consisting of two uppercase letters, extended length (4 bytes) is
+     *  assumed, since it could be a new DICOM VR that is not yet supported.
+     *  For other unknown VRs consisting of characters in the range of 32 to
+     *  127, a 2-byte length field is assumed.  This also applies to a VR
+     *  string of "??", which has been observed in the wild not to use extended
+     *  length.  All other VR strings are considered invalid.
      *  @param vrName symbolic name of value representation
      */
     void setVR(const char* vrName);

dcmdata/libsrc/dcvr.cc

@@ -246,14 +246,13 @@ DcmVR::setVR(const char* vrName)
             OFBool found = OFFalse;
             for (int i = 0; i < DcmVRDict_DIM; i++)
             {
-                /* We only compare the first two characters of the passed string. */
-                if ((DcmVRDict[i].vrName[0] == c1) && (DcmVRDict[i].vrName[1] == c2))
+                /* We only compare the first two characters of the passed string
+                 * and never accept a VR that is labeled for internal use only.
+                 */
+                if ((DcmVRDict[i].vrName[0] == c1) && (DcmVRDict[i].vrName[1] == c2) &&
+                    ((DcmVRDict[i].propertyFlags & DCMVR_PROP_INTERNAL) == 0))
                 {
-                    /* Map internal VRs to "unknown" (4-byte length field). */
-                    if (DcmVRDict[i].propertyFlags & DCMVR_PROP_INTERNAL)
-                        vr = EVR_UNKNOWN;
-                    else
-                        vr = DcmVRDict[i].vr;
+                    vr = DcmVRDict[i].vr;
                     found = OFTrue;
                     break;
                 }
@@ -317,7 +316,7 @@ DcmVR::getValidEVR() const
     ** If the generation of post-1993 VRs is not globally enabled then use OB instead.
     ** We may not want to generate these "new" VRs if other software cannot handle it.
     */
-    DcmEVR oldVR = evr;
+    const DcmEVR oldVR = evr;
     switch (evr) {
         case EVR_UN:
             if (!dcmEnableUnknownVRGeneration.get())
@@ -435,8 +434,7 @@ DcmVR::getVRName() const
 const char*
 DcmVR::getValidVRName() const
 {
-    DcmVR avr(getValidEVR());
-    return avr.getVRName();
+    return DcmVR(getValidEVR()).getVRName();
 }
 
 OFBool
@@ -524,7 +522,7 @@ DcmVR::getMaxValueLength() const
 OFBool
 DcmVR::isEquivalent(const DcmVR& avr) const
 {
-    DcmEVR evr = avr.getEVR();
+    const DcmEVR evr = avr.getEVR();
     if (vr == evr) return OFTrue;
 
     OFBool result = OFFalse;