From: cross@spitfire.i.gajendra.net   
      
   In article <10479sq$edc4$1@dont-email.me>,   
   Arne Vajhøj   wrote:   
   >On 7/3/2025 9:00 PM, Craig A. Berry wrote:   
   >> [snip]   
   >> I haven't done any testing of your code.  I do agree with Dan that   
   >> reducing the repeated code would be a benefit.  You don't need the first   
   >> #if because the file won't even be built if HAS_VASPRINTF is detected   
   >> during configuration.   
   >   
   >I know. But just in case it did get called.   
   >   
   >>                       Which means for the remaining 4 cases you've got   
   >> this in all of them that could be put outside the #ifdefs, and without   
   >> any nesting:   
   >>   
   >> va_list cp;   
   >> va_copy(cp, ap);   
   >>    
   >> (len < 0) return len;   
   >>    
   >> *ret = malloc(len + 1);   
   >> if(!*ret) return -1;   
   >> return vsprintf(*ret, fmt, cp);   
   >>   
   >> where something1 is always 1-3 lines and something2 is zero or one line   
   >   
   >There are duplicated lines.   
   >   
   >But I still believe the one block of code per case is more readable   
   >than common code + some case specific code + common code + some   
   >case specific code + common code.   
      
   Do you have a rationale for that belief, or is it entirely   
   subjective?  It's ok if it is, but best to be up front about it   
   if so.   
      
   >Also imagine if someone was to add yet another approach. Now it   
   >is just put in the #if for the case and then write the block of   
   >code. If mixing it would all depend on the new approach - maybe   
   >it would fit into the existing structure, maybe it would require   
   >changing the structure if it does not fit.   
      
   Here's an example, with the "try to vsnprintf into a buffer and   
   just copy if that succeds" approach I mentioned earlier.   
      
   There's no nesting of ifdefs, but because the calculation of the   
   buffer size is decoupled from the logic of allocation and   
   formatting, the latter is simpler.   
      
   	- Dan C.   
      
   /*   
    * SPDX-License-Identifier: LGPL 2.1 OR Apache 2.0   
    *   
    * vasprintf implementation   
    */   
      
   #include    
   #include    
   #include    
   #include    
   #include    
      
   #include "config.h"   
      
   static const size_t FAIL = ~(size_t)0;   
      
   static inline int   
   tds_vsnprintf(char *dst, size_t len, const char *fmt, va_list ap)   
   {   
   #if defined(HAVE_VSNPRINTF)   
   	return vsnprintf(dst, len, fmt, ap);   
   #else   
   	assert(dst != NULL);   
   	assert(len != 0);   
   	return vsprintf(dst, fmt, ap);   
   #endif   
   }   
      
   #if defined(REENTRANT)   
   #define neednull(fp) 1   
   #define putnull(fp) fclose(fp)   
   #else   
   #define neednull(fp) ((fp) == NULL)   
   #define putnull(fp)   
   #endif   
      
   static inline size_t   
   tds_vasprintf_bufsize(const char *fmt, va_list ap)   
   {   
   	int len;   
   #if defined(HAVE_VSCPRINTF)   
   	len = _vscprintf(fmt, ap);   
   #elif defined(HAVE_VSNPRINTF)   
   	len = vsnprintf(NULL, 0, fmt, ap);   
   #else   
   	static FILE *fp = NULL;   
   	if (neednull(fp))   
   		fp = fopen(TDS_PATH_DEVNULL, "w");   
   	if (fp == NULL)   
   		return FAIL;   
   	len = vfprintf(fp, fmt, ap);   
   	putnull(fp);   
   #endif   
   	if(len < 0)   
   		return FAIL;   
   	return (size_t)len + 1;   
   }   
      
   int   
   tds_vasprintf(char **outp, const char *fmt, va_list ap)   
   {   
   #if defined(HAVE_VASPRINTF)   
   	return vasprintf(outp, fmt, ap);   
   #elif !defined(va_copy)   
   #error "va_copy macro required"   
   #endif   
   	va_list save_ap;   
   	va_copy(save_ap, ap);   
   	char *dst;   
   	size_t len;   
   	int rv;   
   #if defined(HAVE_VSNPRINTF)   
   	char buf[1024];   
   	rv = vsnprintf(buf, sizeof(buf), fmt, ap);   
   	if (rv < 0)   
   		return -1;   
   	len = (size_t)rv + 1;   
   	if (len <= sizeof(buf)) {   
   		dst = malloc(len);   
   		if (dst == NULL)   
   			return -1;   
   		memcpy(dst, buf, len);   
   		*outp = dst;   
   		return rv;   
   	}   
   #else   
   	len = tds_vasprintf_bufsize(fmt, ap);   
   #endif   
   	if (len == FAIL)   
   		return -1;   
   	dst = malloc(len);   
   	if (dst == NULL)   
   		return -1;   
   	rv = tds_vsnprintf(dst, len, fmt, save_ap);   
   	*outp = dst;   
   	return rv;   
   }   
      
   #if defined(TEST)   
      
   #include    
   #include    
      
   char *   
   dovasprintf(const char *fmt, ...)   
   {   
   	va_list ap;   
   	va_start(ap, fmt);   
   	char *out = NULL;   
   	int rv = tds_vasprintf(&out, fmt, ap);   
   	va_end(ap);   
   	if (rv < 0)   
   		return NULL;   
   	return out;   
   }   
      
   int   
   main()   
   {   
   	char *p = dovasprintf("This is the %dth test: %s", 10, "test string");   
   	assert(strcmp(p, "This is the 10th test: test string") == 0);   
   	return EXIT_SUCCESS;   
   }   
      
   #endif   
      
   --- SoupGate-Win32 v1.05   
    * Origin: you cannot sedate... all the things you hate (1:229/2)