[gdal-dev] VS2010-VS2013 s57

[Even Rouault]

Le mercredi 04 décembre 2013 22:15:58, Dmitriy Baryshnikov a écrit :
> Hi Frank,
> 
> You quite right, what
> 
>     It is not clear to me why this should be necessary.  osAcronym is a
>     long lived std::string (well CPLString derived from std::string)
>     living in the registrars attribute list.  I assume the following
>     method is used to convert the CPLString to "const char *" which
>     should amount to the same thing you did.
> 
> but this is not helps, as c_str() method of std::string not return
> pointer, but construct new zero ended const char* managed by std::string
> (maybe this is specific to VS2010-VS2013). This const char* cleared
> after some call to std::string or CPLString and we got empty (cleared)
> const char *pszAcronym. I think the VS2010-VS2013 compiler do this in
> getter by index, operator const char* or the whole GetAttrInfo(i) ==
> NULL ? NULL : aoAttrInfos[i]->osAcronym (maybe MS change the order
> parameter calculation or optimization). Maybe const (operator const
> char* (void) const) in non const (const char *GetAttrAcronym( int i ))
> function VS2010-VS2013 specific behaviour.
> 
> OK, I fire the ticket. As I have committer rights, my I add changes,
> corresponding to the ticket?

Yes, Dmitriy, please commit your change (GetAttrName() would need the same 
change also). Hopefully, it will solve the failure on ogr_s57_6 that have been 
seen in http://www.gisinternals.com/sdk/build-output/vc10-20131204-5-52-50-84-
vc10-dev.txt since a few weeks and I couldn't understand.
And actually this must be related to one of my latest change in the S57 driver 
where I've restructured that area a bit to gain thread-safety.
I'm not sure if VS2010 is right or wrong in doing what it does, but if the 
.c_str() fix the issue, that's always good to have !

Even

> 
> Best regards,
>      Dmitry
> 
> 05.12.2013 0:19, Frank Warmerdam пишет:
> > Dmitriy,
> > 
> > It is not clear to me why this should be necessary.  osAcronym is a
> > long lived std::string (well CPLString derived from std::string)
> > living in the registrars attribute list.  I assume the following
> > method is used to convert the CPLString to "const char *" which should
> > amount to the same thing you did.
> > 
> >     operator const char* (void) const { return c_str(); }
> > 
> > Perhaps there is some subtle reason I don't see that the compiler is
> > creating a temporary std::string in between?
> > 
> > In any event, if you file a ticket I can apply this change upstream.
> > 
> >  There are other accessors on the same class that look like they could
> > 
> > have similar issues.
> > 
> > Best regards,
> > Frank
> > 
> > 
> > On Wed, Dec 4, 2013 at 12:11 PM, Dmitriy Baryshnikov
> > 
> > <bishop.dev at gmail.com <mailto:bishop.dev at gmail.com>> wrote:
> >     Hi,
> >     
> >     I have such error: the GDAL compiled with VS2010-VS2013 in s57
> >     driver loose all additional fields values. But the same code
> >     compiled with gcc or previous VS works fine.
> >     I found the root of problems here
> >     
> >     (ogr\ogrsf_frmts\s57\s57reader.cpp:932):
> >         const char *pszAcronym = poRegistrar->GetAttrAcronym(nAttrId);
> >         iField = poFeature->GetDefnRef()->GetFieldIndex(pszAcronym);
> >     
> >     The pszAcronym always empty.
> >     
> >     The problem comes from this function (ogr\ogrsf_frmts\s57\s57.h:140):
> >         const char *GetAttrAcronym( int i )
> >         
> >             { return GetAttrInfo(i) == NULL ? NULL :
> >         aoAttrInfos[i]->osAcronym; }
> >     
> >     It seems to me that during execution this function I have
> >     
> >     |situation when||c_str()|result becomes invalid
> >     
> >     (the|std::string|is destroyed or a non-const member function of
> >     the string is called).
> >     
> >     If I change function
> >     
> >         const char *GetAttrAcronym( int i )
> >         
> >             { return GetAttrInfo(i) == NULL ? NULL :
> >         aoAttrInfos[i]->osAcronym.c_str(); }
> >     
> >     the problem gone.
> >     
> >     So I need some confirmation/verification my ideas.
> >     If I'm right, I can make changes to s57 driver.
> >     
> >     Best regards,
> >     
> >          Dmitry
> >     
> >     ---------------------------------------------------------------------
> >     --- <http://www.avast.com/>
> >     
> >     Это сообщение свободно от вирусов и вредоносного ПО благодаря
> >     avast! Antivirus <http://www.avast.com/> защита активна.
> >     
> >     
> >     
> >     _______________________________________________
> >     gdal-dev mailing list
> >     gdal-dev at lists.osgeo.org <mailto:gdal-dev at lists.osgeo.org>
> >     http://lists.osgeo.org/mailman/listinfo/gdal-dev
> 
> ---
> Это сообщение свободно от вирусов и вредоносного ПО благодаря защите от
> вирусов avast! http://www.avast.com

-- 
Geospatial professional services
http://even.rouault.free.fr/services.html