#5907 closed enhancement (fixed)
Replaces strdup() / free() by CPLStrdup() / CPLFree()
Reported by: | yzhong | Owned by: | warmerdam |
---|---|---|---|
Priority: | normal | Milestone: | 2.0.0 |
Component: | default | Version: | svn-trunk |
Severity: | normal | Keywords: | strdup |
Cc: | tamas |
Description
The strdup() function is deprecated under Windows and not safe to use. I have replace a large number of strdup() to CPLStrdup(), and their corresponding free() to CPLFree().
There are a few left, which I am not too sure whether changing will cause other problems. Therefore, I leave them untouched. They are in the following files:
gdal\frmts\pcidsk\sdk\pcidsk_shape.h gdal\ogr\ogrsf_frmts\geojson\libjson\json_object.c gdal\port\cpl_vsisimple.cpp
If you do a global search, you will find them all.
Also, in the original code, I notice that some memory that are allocated by strdup() are released by CPLFree(). Therefore, if you only look at the changes according to the the patch file, CPLStrdup() and CPLFree() don't always come in pair.
I understand that there might be cases where memory is allocated by CPLStrdup() but released by free() in the original code. I don't have time to go through all these yet because there are just too many free() calls in the entire project. But I assume that it is fine since we have define to CPLFree to free.
#define CPLFree free
Attachments (1)
Change History (6)
by , 9 years ago
Attachment: | patches.zip added |
---|
comment:1 by , 9 years ago
comment:2 by , 9 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
Ah, just seeing @warmerdam comment now I've committed... We had a discussion with yzhong on https://github.com/OSGeo/gdal/pull/51 and apparently the use of strdup() causes heap memory corruption for some reason which aren't clear to me either. Hopefull the changes will not cause harm.
trunk r28831 "Replaces strdup() / free() by CPLStrdup() / CPLFree() to avoid issues on Windows with newer MSVC versions (derived patch by yzhong + extra fixes from mine, #5907)"
I dropped the change in libjson-c as it is an external library whose code is imported in GDAL, so change in it should be contributed to the upstream project (which doesn't use GDAL CPLxxx routines)
comment:3 by , 9 years ago
From https://github.com/OSGeo/gdal/pull/55 : trunk r28971 "VSIStrdup(): avoid using strdup() (#5907)"
comment:4 by , 9 years ago
Cc: | added |
---|
Tamas, I was wondering if you'd have any clue why strdup() would suddenly cause problems with newer MSVC versions (VS2013 apparently). I haven't yet seen a really convincing explanation, although apparently changing strdup() by malloc()+strcpy() seems to improves things for the reporter.
IMHO the so called deprecation of strdup() in MSVC is no reason for us to stop using it. I am dubious about the value of this change.