This is a repost from the mailinglist, as developing discussion was not supposed to be there (my fault).
Please leave some comments or flames.
-----
Long time tiny user, first time on the list.
Some background.
We are using tiny on a few closed embedded devices (arm, hitachi) and on some embedded x86 devices. Been using it for several years now.
I have a few suggestions for the TIXML_SAFE newly introduced.
Refering to 2.4.2 release in my comments below.
First of all.
Its not that nice to check if TIXML_SNPRINTF is defined everytime in the source (like tinexml.cpp:104). Better to always use TIXML_SNPRINTF and for those platforms that don't have safe calls (like a few of mine) lets do a unsafe conversion.
Example code (don't know if it compiles as i write it first here in the mail)
#if defined(_MSC_VER) && (_MSC_VER >= 1200 )
// Microsoft visual studio, version 6 and higher.
//#pragma message( "Using _sn* functions." )
#define TIXML_SNPRINTF _snprintf
#define TIXML_SNSCANF _snscanf
#elif defined(__GNUC__) && (__GNUC__ >= 3 )
// GCC version 3 and higher.s
//#warning( "Using sn* functions." )
#define TIXML_SNPRINTF snprintf
#define TIXML_SNSCANF snscanf
#else
#define TIXML_SNPRINTF ti_snprintf
#define TIXML_SNSCANF ti_snscanf
#endif
extern "C" int ti_snprintf(char *s, size_t n, const char* str, ...)
{
va_list args;
va_start(args, str);
int result = vsprintf(s, str, args);
// Not good (dont forget to count the zero termination)
assert(strlen(s) < n);
va_end(args);
return result;
}
Secondly.
You probably always want to use safe sprints & scans if they are available. Therefore the define TIXML_SAFE defined in the tinyxml.h should be removed completely.
My two cents. Comments & flames are welcome.
/MS
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
On the first point, I agree the extra #ifdef is messy, but the extra (useless) function seems a bit verbose too.
On the 2nd point, the safe functions aren't in the standard library. This is a big problem, because they aren't guarenteed, by any means, to actually be available.
lee
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
#1
the 'extra function' is not completely useless. It's a little verbose as you mentioned but it provides some safeness for unsafe enviroments (its asserting on overflow).
#2
Thats there the 'extra function' does the trick. On enviroments without safe functions this is a fallback to standard library functions.
/MS
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
This shows how to use safe functions when available and use 'built in' safe functions when those are not available.
This is not final in anyway but its shows what I think could be a better solution.
I'm willing to extend this patch, to include all deprecated functions and build corresponding functions for enviroments there those are not available, if you are interested.
/MS
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
This is a repost from the mailinglist, as developing discussion was not supposed to be there (my fault).
Please leave some comments or flames.
-----
Long time tiny user, first time on the list.
Some background.
We are using tiny on a few closed embedded devices (arm, hitachi) and on some embedded x86 devices. Been using it for several years now.
I have a few suggestions for the TIXML_SAFE newly introduced.
Refering to 2.4.2 release in my comments below.
First of all.
Its not that nice to check if TIXML_SNPRINTF is defined everytime in the source (like tinexml.cpp:104). Better to always use TIXML_SNPRINTF and for those platforms that don't have safe calls (like a few of mine) lets do a unsafe conversion.
Example code (don't know if it compiles as i write it first here in the mail)
#if defined(_MSC_VER) && (_MSC_VER >= 1200 )
// Microsoft visual studio, version 6 and higher.
//#pragma message( "Using _sn* functions." )
#define TIXML_SNPRINTF _snprintf
#define TIXML_SNSCANF _snscanf
#elif defined(__GNUC__) && (__GNUC__ >= 3 )
// GCC version 3 and higher.s
//#warning( "Using sn* functions." )
#define TIXML_SNPRINTF snprintf
#define TIXML_SNSCANF snscanf
#else
#define TIXML_SNPRINTF ti_snprintf
#define TIXML_SNSCANF ti_snscanf
#endif
extern "C" int ti_snprintf(char *s, size_t n, const char* str, ...)
{
va_list args;
va_start(args, str);
int result = vsprintf(s, str, args);
// Not good (dont forget to count the zero termination)
assert(strlen(s) < n);
va_end(args);
return result;
}
Secondly.
You probably always want to use safe sprints & scans if they are available. Therefore the define TIXML_SAFE defined in the tinyxml.h should be removed completely.
My two cents. Comments & flames are welcome.
/MS
On the first point, I agree the extra #ifdef is messy, but the extra (useless) function seems a bit verbose too.
On the 2nd point, the safe functions aren't in the standard library. This is a big problem, because they aren't guarenteed, by any means, to actually be available.
lee
#1
the 'extra function' is not completely useless. It's a little verbose as you mentioned but it provides some safeness for unsafe enviroments (its asserting on overflow).
#2
Thats there the 'extra function' does the trick. On enviroments without safe functions this is a fallback to standard library functions.
/MS
I've created a patch:
1350196 Safe tiny - review 1
This shows how to use safe functions when available and use 'built in' safe functions when those are not available.
This is not final in anyway but its shows what I think could be a better solution.
I'm willing to extend this patch, to include all deprecated functions and build corresponding functions for enviroments there those are not available, if you are interested.
/MS