Why ISC developing wheel again?

Adam Tkac atkac at redhat.com
Fri Oct 5 09:14:12 UTC 2007


On Fri, Oct 05, 2007 at 10:40:47AM +1000, Mark Andrews wrote:
> 
> > > 	isc_print_snprintf() is there only for those platforms where
> > > 	configure cannot find a working (C 99) snprintf().  We do
> > > 	have people that compile named on such platforms.  The code
> > > 	to do this was added circa Jun '03.  If we find a working
> > > 	sprintf() we use it.  Similarly for vsnprintf().
> > 
> > try compile nsupdate and then
> > 
> > $nm nsupdate |grep isc_string_printf
> > 08140d40 T isc_string_printf
> > 08140c80 T isc_string_printf_truncate
> 
> 	isc_string_printf and isc_string_printf_truncate are NOT
> 	the private implenentations of sprintf.  isc_print_sprintf()
> 	is the private implementation of sprintf()

I'm not talking about sprintf() implementation. I'm talking about snprintf (size of buffer is specified). Generally using of sprintf is very bad. snprintf should be used instead.

>  
> > > AC_MSG_CHECKING(sprintf)
> > > AC_TRY_COMPILE([
> > > #include <stdio.h>
> > > ],
> > > [ char buf[2]; return(*sprintf(buf,"x"));],
> > > [
> > > ISC_PRINT_OBJS="print.$O"
> > > ISC_PRINT_SRCS="print.c"
> > > ISC_PLATFORM_NEEDSPRINTF="#define ISC_PLATFORM_NEEDSPRINTF"
> > > LWRES_PLATFORM_NEEDSPRINTF="#define LWRES_PLATFORM_NEEDSPRINTF"
> > > ],
> > > [ISC_PLATFORM_NEEDSPRINTF="#undef ISC_PLATFORM_NEEDSPRINTF"
> > >  LWRES_PLATFORM_NEEDSPRINTF="#undef LWRES_PLATFORM_NEEDSPRINTF"]
> > > )
> > > 
> > > 	isc_string_printf() and isc_string_printf_truncate() both
> > > 	call vsnprintf() and enforce additional semantics, i.e. no
> > > 	accidental truncation of the output.
> > 
> > Yes, this looks fine. But I don't know why you're using isc_string_printf fun
> > ction in nsupdate.c directly?
> 
> 	isc_string_printf != isc_print_sprintf
> 
> 	This is isc_string_printf().  It is NOT a replacement for missing
> 	C99 functionality.
> 
> 	[It also need the terminating \0 to be added in the error handling]
> 
> isc_result_t
> isc_string_printf(char *target, size_t size, const char *format, ...) {
>         va_list args;
>         size_t n;
> 
>         REQUIRE(size > 0U);
> 
>         va_start(args, format);
>         n = vsnprintf(target, size, format, args);
>         va_end(args);
> 
>         if (n >= size) {
>                 memset(target, ISC_STRING_MAGIC, size);
>                 return (ISC_R_NOSPACE);
>         }
> 
>         ENSURE(strlen(target) < size);
> 
>         return (ISC_R_SUCCESS);
> }
> 

You're right that isc_string_printf != isc_print_sprintf. But I still don't know why you need both functions (nearly same). I think you should chose one of them and use only that function. Functionality - same, code lucidy - better. But you're maintainer and I'm not going to blame you for your decision.

> -- 
> Mark Andrews, ISC
> 1 Seymour St., Dundas Valley, NSW 2117, Australia
> PHONE: +61 2 9871 4742                 INTERNET: Mark_Andrews at isc.org

Adam



More information about the bind-workers mailing list