From: Charles S. <ham...@gm...> - 2009-11-13 22:33:48
|
Stelios, Thank you for the comments. Some are obvious errors, others confuse me, and others seem style questions. Perhaps I should have explained my reasoning when posting the patch but that is a lesson for the future. On Thu, Nov 12, 2009 at 10:53 PM, Stelios Bounanos <sb...@en...>wrote: > >>>>> On Thu, 12 Nov 2009 20:00:52 -0500, Charles Suprin < > ham...@gm...> said: > > > Index: dummy/dummy.c > > =================================================================== > > --- dummy/dummy.c (revision 2768) > > +++ dummy/dummy.c (working copy) > > @@ -40,6 +40,9 @@ > > > #include "dummy.h" > > > +#define SAFER_SPRINTF(x,y,z) {if > (snprintf((x),sizeof(x),(y),(z))==sizeof(x)) {return -RIG_EINTERNAL;}} > > #define SAFER_SPRINTF(x, ...) {if > (snprintf((x),sizeof(x),__VA_ARGS__)>=sizeof(x)) {return -RIG_EINTERNAL;}} > > so that it will work with more than two arguments (note the >= > too). Also it only works for arrays. > > There are several points here, so I will try to address them one at a time. Yes this only works for arrays. This is all that is required here. However it would be nice to have something that worked for a more general solution. It also would be nice if someone tried to use a pointer the compiler would throw a processor exception. However I am not a master of the preprocessor judo. If this were C++ it would not be an issue, but here we are and I do not have a nicer solution. The use of ellipsis here (...) was something I could not decide whether it was the right answer or not. It was not clear whether for portability and security it was worthwhile. None of the applications of this macro required it and I have worked with some embedded compiles that do not support it. (...) came about in C99. Security is a little bit of a weaker position, but if the compiler throws an error if someone uses it incorrectly it seems a better idea. Again, the right answer is not obvious to me. > +#define SAFER_STRCPY(x,y) {if (strlen(y)>sizeof(x)) {return > -RIG_EINTERNAL;} else {strncpy((x),(y),sizeof(x));}} > > Again you'd have to be careful to never use this when x is a char* and > not a char[]. If it must work with pointers you have to check strlen(y) > and strlen(x), and then you might as well use memcpy for the copying. > > [snip] > Again, same discussion with char* versus char[] as above. Now on the use of strlen I am confused. strlen only tells you how many characters are in the order before it reaches the string terminatino character. This may be before the end of the buffer or after the end of the buffer. If in c++ we could query the length of the string buffer but this being c it needs to be passed. > > > +extern HAMLIB_EXPORT(int) snprintf_freq(char *str, freq_t,int); > > IMO this should be snprintf_freq(char *str, size_t, freq_t), i.e., it > should mimic the snprintf prototype. BTW, size_t is in stddef.h. > > > IMO you are absolutely correct on this one. Respectfully, Charles AA1VS |