[Dcmlib] Incoherence de longueur de champ

[Mathieu Malaterre]

Eric Boix wrote:
> 	Yo,
> 
> 
>>0008|0050 lg :       x(2) 2        Off.:     x(1bc) 444     [SH] 
>>                                           [Accession Number] []
>>
>>Mais j'ai beau chercher il ne passe pas dans LoadDocEntry. Comment a 
>>partir d'un group element je trouve a quoi il correspond (DocEntry, 
>>ValEntry, SeqEntry ... )
> 
> 
> Morceaux choisis de void Document::ParseDES():
> 
> {
>    ...
>    if ( vr != "SQ" )
>    {
>       if ( Global::GetVR()->IsVROfGdcmStringRepresentable(vr) )
>       {
>          /////////////////////// ValEntry
>          ValEntry* newValEntry = new ValEntry(...);
>          ...
>       } else {
>          //////////////////// BinEntry or UNKOWN VR:
>          BinEntry* newBinEntry = new BinEntry(...);
>       }
>       ...
>    } else {
>       // VR = "SQ"
>       SeqEntry* newSeqEntry = new SeqEntry(...)
>       ...
>    }          
>    ...
> }
> 
> Moralite':
>   * si vr = 'sq' c'est une sequence (et on recurse).
>   * si vr est "StringRepresentable" c'est une ValEntry (on interprete
>     les donnees binaires comme un string),
>   * si vr n'est pas "StringRepresentable" c'est donc du binaire
> 
> A noter que pour un (group, element) qui n'est pas dans le dictionaire
> la VR est dans le fichier...
> 
> C,a va ?


Bon ca va, j'ai dormi. J'ai place qlq assert et j'ai trouve' le 
probleme: (gdcmDocument.cxx), ligne 1681 (*)

extrait:
    Fp->read(str, (size_t)length);
    str[length] = '\0';
    std::string newValue = str;

Qlq plaisir de la std::string. Il y a une *enorme* difference entre:

const char *s = "\0\0";
std::string a = s;

-> a.size() = 0 !!!!

*Mais*

const char *s = "\0\0";
std::string a(s,s+2); // will copy 2 '\0'

-> a.size() = 2

et ou peut utiliser c_str() / data() indifferemment. Puisque la taille 
de la string sera la bonne.


Donc je voudrais ajouter une notion de DicomString dans gdcmUtil:



/**
  * \ingroup Util
  * \brief Create a /DICOM/ string:
  * It should a of even lenght (no odd lenght ever)
  * It can contains as many \0 as you want.
  * This function is similar to DicomString(const char*), except it doesn't
  * take a lenght. It only pad with a null character if legnth is odd
  */
std::string Util::DicomString(const char* s)
{
    size_t l = strlen(s);
    if( l%2 )
    {
       l++;
    }
    std::string r(s, s+l);
    assert( !(r.size() % 2) );
    return r;
}

/**
  * \ingroup Util
  * \brief Create a /DICOM/ string:
  * It should a of even lenght (no odd lenght ever)
  * It can contains as many \0 as you want.
  */
std::string Util::DicomString(const char* s, size_t l)
{
    std::string r(s, s+l);
    assert( !(r.size() % 2) );
    return r;
}


L'avantage c'est qu'ensuite au moment de l'ecriture on peut toujours 
comparer la taille de la string par rapport a GetReadLenght() et faire 
un assert (j'aime bien les assert car ca na pas de cout en Release). Je 
pense que la classe gdcmValEntry doit vraiment avoir la notion de 
'DicomString', pas de chaine ayant un nombre impaire de caractere. Pour 
l'instant je ne peux pas obliger ca a cause des chaines du genre:

"19785\8257\17747\8273"

J'aurais preferer une notion de gdcmValVectorEntry...

Mathieu
Ps: j'ai bine l'impression que l'on ecrivait sur le disque de la memoire 
aleatoire jusqu'a present. Et ca passait car std::string en Debug doit 
mettre un packet de zero sous linux, mais par sous win32:

http://public.kitware.com/Public/Sites/DASH3.kitware/GDCM-Win32-vs60/20041105-0100-Nightly/Test.html


(*)
   // We need an additional byte for storing \0 that is not on disk
    char *str = new char[length+1];
    Fp->read(str, (size_t)length);
    str[length] = '\0';
    std::string newValue = str;
    delete[] str;

    if ( ValEntry* valEntry = dynamic_cast<ValEntry* >(entry) )
    {
       if ( Fp->fail() || Fp->eof())//Fp->gcount() == 1
       {
          dbg.Verbose(1, "Document::LoadDocEntry",
                         "unread element value");
          valEntry->SetValue(GDCM_UNREAD);
          return;
       }

       if( vr == "UI" )
       {
          // Because of correspondance with the VR dic
          valEntry->SetValue(newValue);
       }
       else
       {
          valEntry->SetValue(newValue);
       }
    }