String Safety

[mh]

Windows 95 had to run in 4mb I believe, although I've never known anyone successfully do so. I did see it running in 8mb once and it was horrible.

The main flaws with Windows 95 were a combination of design decisions made to squeeze it into this tiny space, it's DOS heritage, maintaining an extremely high level of backwards compatibility, and the way it mixed 16-bit and 32-bit code. Much of these were valid design decisions or for reasons that were correct at the time, but that doesn't mean that they're still valid or correct today. Many of the (oh so true) criticisms of Windows from the Unix side were based on this stuff, and completely disappeared with NT-based versions (many in the Unix side seem unaware of this and still rely on outdated criticisms).

Windows 98 was an over-engineered follow-on that added lots of extra stuff on top but didn't do much (if anything) to fix some of the glaring problems underneath. FAT32 and USB support were good to have though. I think history will show it as more a stringing-out of the old DOS-based Windows line than a viable product in it's own right (definitely true with ME); if MS had been ready with an NT5 version that was suitable for home use at the time, or even a little later, it may have never even existed.

DOS-based Windows was effectively already dead by the time Windows 95 was released. NT had existed for some years, and was clearly superior in just so many ways.

[Spike]

fat32 and usb were in win95b. perhaps not many class drivers, but enough to get a usable system hence the prevelence of non-class devices at the time.
from my experience win95's bugs were generally fairly predictable. win98 always seemed so random when it crashed.
active desktop and pretty window titlebars are the only two things win98 really offered over 95...

[mh]

I'd forgotten about 95b.

You could also get a large chunk of the 98 experience by installing IE4 on 95; all the extra desktop gunk and all the random crashes, heavy page-file usage and other nonsense.

My favourite was the way the OS used to blue-screen if you ejected a CD while it was still being read.

[revelator]

actually 95 could run on 2 mb but you needed to trick the installer unless you ran the install on a seperate machine. it was massively slow though and only bearable with the use of memory compression tools like QEMM.
and aye usb support allready came with 95 in my case i got it with the OSR2 version. though XP couldnt handle a lot of dos based games out of the box i could usually trick them into running by using tools like VDM sound etc.
still got my old WTF PC i used to pull a trick on peeps back then with, it was built into a massive case from an 286 but was actually an overclocked celeron 300A. usually this cpu ran at 300 mhz but i managed to overclock it to 600 so 100% OC
it still runs btw hehe. back then this beast could outrun even very expensive machines so thats where the WTF came from

[revelator]

something i cooked up from the BSD print buffers. These correctly nulls the string but are a bit massive.

Code: Select all

/* BSD print buffers, correctly nulled out */
#ifndef HAVE_BSD_SNPRINTF

#ifndef VA_COPY
#define VA_COPY(dest, src) (dest) = (src)
#endif

#ifdef _WIN64
# define LDOUBLE long double
#else
# define LDOUBLE double
#endif

#ifdef _WIN64
# define LLONG long long
#else
# define LLONG long
#endif

/*
 * dopr(): poor man's version of doprintf
 */

/* format read states */
#define DP_S_DEFAULT 0
#define DP_S_FLAGS   1
#define DP_S_MIN     2
#define DP_S_DOT     3
#define DP_S_MAX     4
#define DP_S_MOD     5
#define DP_S_CONV    6
#define DP_S_DONE    7

/* format flags - Bits */
#define DP_F_MINUS 		(1 << 0)
#define DP_F_PLUS		(1 << 1)
#define DP_F_SPACE 		(1 << 2)
#define DP_F_NUM   		(1 << 3)
#define DP_F_ZERO  		(1 << 4)
#define DP_F_UP    		(1 << 5)
#define DP_F_UNSIGNED 	(1 << 6)

/* Conversion Flags */
#define DP_C_SHORT   1
#define DP_C_LONG    2
#define DP_C_LDOUBLE 3
#define DP_C_LLONG   4

#define char_to_int(p) ((p)- '0')
#ifndef MAX
# define MAX(p,q) (((p) >= (q)) ? (p) : (q))
#endif

#define DOPR_OUTCH(buf, pos, buflen, thechar) \
	do { \
		if (pos + 1 >= INT_MAX) { \
			errno = ERANGE; \
			return -1; \
		} \
		if (pos < buflen) \
			buf[pos] = thechar; \
		(pos)++; \
	} while (0)

static _inline int dopr(char *buffer, size_t maxlen, const char *format, va_list args_in);
static _inline int fmtstr(char *buffer, size_t *currlen, size_t maxlen, char *value, int flags, int min, int max);
static _inline int fmtint(char *buffer, size_t *currlen, size_t maxlen, LLONG value, int base, int min, int max, int flags);
static _inline int fmtfp(char *buffer, size_t *currlen, size_t maxlen, LDOUBLE fvalue, int min, int max, int flags);

static _inline int dopr(char *buffer, size_t maxlen, const char *format, va_list args_in)
{
	char	ch;
	LLONG	value;
	LDOUBLE fvalue;
	char	*strvalue;
	int		min;
	int		max;
	int		state;
	int		flags;
	int		cflags;
	size_t	currlen;
	va_list args;

	VA_COPY(args, args_in);
	
	state = DP_S_DEFAULT;
	currlen = flags = cflags = min = 0;
	max = -1;
	ch = *format++;
	
	while (state != DP_S_DONE) 
	{
		if (ch == '\0') 
			state = DP_S_DONE;

		switch(state) {
		case DP_S_DEFAULT:
			if (ch == '%') 
				state = DP_S_FLAGS;
			else
				DOPR_OUTCH(buffer, currlen, maxlen, ch);
			ch = *format++;
			break;
		case DP_S_FLAGS:
			switch (ch) {
			case '-':
				flags |= DP_F_MINUS;
				ch = *format++;
				break;
			case '+':
				flags |= DP_F_PLUS;
				ch = *format++;
				break;
			case ' ':
				flags |= DP_F_SPACE;
				ch = *format++;
				break;
			case '#':
				flags |= DP_F_NUM;
				ch = *format++;
				break;
			case '0':
				flags |= DP_F_ZERO;
				ch = *format++;
				break;
			default:
				state = DP_S_MIN;
				break;
			}
			break;
		case DP_S_MIN:
			if (isdigit((unsigned char)ch)) {
				min = 10*min + char_to_int (ch);
				ch = *format++;
			} else if (ch == '*') {
				min = va_arg (args, int);
				ch = *format++;
				state = DP_S_DOT;
			} else {
				state = DP_S_DOT;
			}
			break;
		case DP_S_DOT:
			if (ch == '.') {
				state = DP_S_MAX;
				ch = *format++;
			} else { 
				state = DP_S_MOD;
			}
			break;
		case DP_S_MAX:
			if (isdigit((unsigned char)ch))	{
				if (max < 0)
					max = 0;
				max = 10*max + char_to_int (ch);
				ch = *format++;
			} else if (ch == '*') {
				max = va_arg (args, int);
				ch = *format++;
				state = DP_S_MOD;
			} else {
				state = DP_S_MOD;
			}
			break;
		case DP_S_MOD:
			switch (ch) {
			case 'h':
				cflags = DP_C_SHORT;
				ch = *format++;
				break;
			case 'l':
				cflags = DP_C_LONG;
				ch = *format++;
				if (ch == 'l') {	
					/* It's a long long */
					cflags = DP_C_LLONG;
					ch = *format++;
				}
				break;
			case 'L':
				cflags = DP_C_LDOUBLE;
				ch = *format++;
				break;
			default:
				break;
			}
			state = DP_S_CONV;
			break;
		case DP_S_CONV:
			switch (ch) {
			case 'd':
			case 'i':
				if (cflags == DP_C_SHORT) 
					value = va_arg (args, int);
				else if (cflags == DP_C_LONG)
					value = va_arg (args, long int);
				else if (cflags == DP_C_LLONG)
					value = va_arg (args, LLONG);
				else
					value = va_arg (args, int);
				if (fmtint(buffer, &currlen, maxlen,
				    value, 10, min, max, flags) == -1)
					return -1;
				break;
			case 'o':
				flags |= DP_F_UNSIGNED;
				if (cflags == DP_C_SHORT)
					value = va_arg (args, unsigned int);
				else if (cflags == DP_C_LONG)
					value = (long)va_arg (args, unsigned long int);
				else if (cflags == DP_C_LLONG)
					value = (long)va_arg (args, unsigned LLONG);
				else
					value = (long)va_arg (args, unsigned int);
				if (fmtint(buffer, &currlen, maxlen, value,
				    8, min, max, flags) == -1)
					return -1;
				break;
			case 'u':
				flags |= DP_F_UNSIGNED;
				if (cflags == DP_C_SHORT)
					value = va_arg (args, unsigned int);
				else if (cflags == DP_C_LONG)
					value = (long)va_arg (args, unsigned long int);
				else if (cflags == DP_C_LLONG)
					value = (LLONG)va_arg (args, unsigned LLONG);
				else
					value = (long)va_arg (args, unsigned int);
				if (fmtint(buffer, &currlen, maxlen, value,
				    10, min, max, flags) == -1)
					return -1;
				break;
			case 'X':
				flags |= DP_F_UP;
			case 'x':
				flags |= DP_F_UNSIGNED;
				if (cflags == DP_C_SHORT)
					value = va_arg (args, unsigned int);
				else if (cflags == DP_C_LONG)
					value = (long)va_arg (args, unsigned long int);
				else if (cflags == DP_C_LLONG)
					value = (LLONG)va_arg (args, unsigned LLONG);
				else
					value = (long)va_arg (args, unsigned int);
				if (fmtint(buffer, &currlen, maxlen, value,
				    16, min, max, flags) == -1)
					return -1;
				break;
			case 'f':
				if (cflags == DP_C_LDOUBLE)
					fvalue = va_arg (args, LDOUBLE);
				else
					fvalue = va_arg (args, double);
				if (fmtfp(buffer, &currlen, maxlen, fvalue,
				    min, max, flags) == -1)
					return -1;
				break;
			case 'E':
				flags |= DP_F_UP;
			case 'e':
				if (cflags == DP_C_LDOUBLE)
					fvalue = va_arg (args, LDOUBLE);
				else
					fvalue = va_arg (args, double);
				if (fmtfp(buffer, &currlen, maxlen, fvalue,
				    min, max, flags) == -1)
					return -1;
				break;
			case 'G':
				flags |= DP_F_UP;
			case 'g':
				if (cflags == DP_C_LDOUBLE)
					fvalue = va_arg (args, LDOUBLE);
				else
					fvalue = va_arg (args, double);
				if (fmtfp(buffer, &currlen, maxlen, fvalue,
				    min, max, flags) == -1)
					return -1;
				break;
			case 'c':
				DOPR_OUTCH(buffer, currlen, maxlen, va_arg (args, int));
				break;
			case 's':
				strvalue = va_arg (args, char *);
				if (!strvalue) strvalue = "(NULL)";
				if (max == -1) {
					max = strlen(strvalue);
				}
				if (min > 0 && max >= 0 && min > max) max = min;
				if (fmtstr(buffer, &currlen, maxlen, strvalue, flags, min, max) == -1)
					return -1;
				break;
			case 'p':
				strvalue = va_arg (args, void *);
				if (fmtint(buffer, &currlen, maxlen, (long) strvalue, 16, min, max, flags) == -1)
					return -1;
				break;
			case 'n':
				if (cflags == DP_C_SHORT) {
					short int *num;
					num = va_arg (args, short int *);
					*num = currlen;
				} else if (cflags == DP_C_LONG) {
					long int *num;
					num = va_arg (args, long int *);
					*num = (long int)currlen;
				} else if (cflags == DP_C_LLONG) {
					LLONG *num;
					num = va_arg (args, LLONG *);
					*num = (LLONG)currlen;
				} else {
					int *num;
					num = va_arg (args, int *);
					*num = currlen;
				}
				break;
			case '%':
				DOPR_OUTCH(buffer, currlen, maxlen, ch);
				break;
			case 'w':
				/* not supported yet, treat as next char */
				ch = *format++;
				break;
			default:
				/* Unknown, skip */
				break;
			}
			ch = *format++;
			state = DP_S_DEFAULT;
			flags = cflags = min = 0;
			max = -1;
			break;
		case DP_S_DONE:
			break;
		default:
			/* hmm? */
			break; /* some picky compilers need this */
		}
	}

	if (maxlen != 0) 
	{
		if (currlen < maxlen - 1) 
		{
			buffer[currlen] = '\0';
		}
		else if (maxlen > 0) 
		{
			buffer[maxlen - 1] = '\0';
		}
	}	
	return currlen < INT_MAX ? (int)currlen : -1;
}

static _inline int fmtstr(char *buffer, size_t *currlen, size_t maxlen, char *value, int flags, int min, int max)
{
	int padlen, strln;     /* amount to pad */
	int cnt = 0;

	if (value == 0) {
		value = "<NULL>";
	}

	for (strln = 0; strln < max && value[strln]; ++strln); /* strlen */
	padlen = min - strln;
	if (padlen < 0) 
		padlen = 0;
	if (flags & DP_F_MINUS) 
		padlen = -padlen; /* Left Justify */
	
	while ((padlen > 0) && (cnt < max)) {
		DOPR_OUTCH(buffer, *currlen, maxlen, ' ');
		--padlen;
		++cnt;
	}
	while (*value && (cnt < max)) {
		DOPR_OUTCH(buffer, *currlen, maxlen, *value);
		*value++;
		++cnt;
	}
	while ((padlen < 0) && (cnt < max)) {
		DOPR_OUTCH(buffer, *currlen, maxlen, ' ');
		++padlen;
		++cnt;
	}
	return 0;
}

/* Have to handle DP_F_NUM (ie 0x and 0 alternates) */
static _inline int fmtint(char *buffer, size_t *currlen, size_t maxlen, LLONG value, int base, int min, int max, int flags)
{
	int				signvalue = 0;
	unsigned LLONG	uvalue;
	char			convert[20];
	int				place = 0;
	int				spadlen = 0; /* amount to space pad */
	int				zpadlen = 0; /* amount to zero pad */
	int				caps = 0;
	
	if (max < 0)
		max = 0;
	
	uvalue = value;
	
	if(!(flags & DP_F_UNSIGNED)) {
		if( value < 0 ) {
			signvalue = '-';
			uvalue = -value;
		} else {
			if (flags & DP_F_PLUS)  /* Do a sign (+/i) */
				signvalue = '+';
			else if (flags & DP_F_SPACE)
				signvalue = ' ';
		}
	}
  
	if (flags & DP_F_UP) caps = 1; /* Should characters be upper case? */

	do {
		convert[place++] =
			(caps? "0123456789ABCDEF":"0123456789abcdef")
			[uvalue % (unsigned)base  ];
		uvalue = (uvalue / (unsigned)base );
	} while(uvalue && (place < 20));
	if (place == 20) place--;
	convert[place] = 0;

	zpadlen = max - place;
	spadlen = min - MAX (max, place) - (signvalue ? 1 : 0);
	if (zpadlen < 0) zpadlen = 0;
	if (spadlen < 0) spadlen = 0;
	if (flags & DP_F_ZERO) {
		zpadlen = MAX(zpadlen, spadlen);
		spadlen = 0;
	}
	if (flags & DP_F_MINUS) 
		spadlen = -spadlen; /* Left Justifty */

	/* Spaces */
	while (spadlen > 0) {
		DOPR_OUTCH(buffer, *currlen, maxlen, ' ');
		--spadlen;
	}

	/* Sign */
	if (signvalue) 
		DOPR_OUTCH(buffer, *currlen, maxlen, signvalue);

	/* Zeros */
	if (zpadlen > 0) {
		while (zpadlen > 0) {
			DOPR_OUTCH(buffer, *currlen, maxlen, '0');
			--zpadlen;
		}
	}

	/* Digits */
	while (place > 0) {
		--place;
		DOPR_OUTCH(buffer, *currlen, maxlen, convert[place]);
	}
  
	/* Left Justified spaces */
	while (spadlen < 0) {
		DOPR_OUTCH(buffer, *currlen, maxlen, ' ');
		++spadlen;
	}
	return 0;
}

static _inline LDOUBLE abs_val(LDOUBLE value)
{
	LDOUBLE result = value;

	if (value < 0)
		result = -value;
	
	return result;
}

static _inline LDOUBLE POW10(int val)
{
	LDOUBLE result = 1;
	
	while (val) {
		result *= 10;
		val--;
	}
  
	return result;
}

static _inline LLONG ROUND(LDOUBLE value)
{
	LLONG intpart;

	intpart = (LLONG)value;
	value = value - intpart;
	if (value >= 0.5) intpart++;
	
	return intpart;
}

/* a replacement for modf that doesn't need the math library. Should
   be portable, but slow */
static _inline double my_modf(double x0, double *iptr)
{
	int		i;
	long	l;
	double	x = x0;
	double	f = 1.0;

	for (i=0;i<100;i++) {
		l = (long)x;
		if (l <= (x+1) && l >= (x-1)) break;
		x *= 0.1;
		f *= 10.0;
	}

	if (i == 100) {
		/*
		 * yikes! the number is beyond what we can handle.
		 * What do we do?
		 */
		(*iptr) = 0;
		return 0;
	}

	if (i != 0) {
		double i2;
		double ret;

		ret = my_modf(x0-l*f, &i2);
		(*iptr) = l*f + i2;
		return ret;
	} 

	(*iptr) = l;
	return x - (*iptr);
}


static _inline int fmtfp (char *buffer, size_t *currlen, size_t maxlen, LDOUBLE fvalue, int min, int max, int flags)
{
	int signvalue = 0;
	double ufvalue;
	char iconvert[311];
	char fconvert[311];
	int iplace = 0;
	int fplace = 0;
	int padlen = 0; /* amount to pad */
	int zpadlen = 0; 
	int caps = 0;
	int idx;
	double intpart;
	double fracpart;
	double temp;
  
	/* 
	 * AIX manpage says the default is 0, but Solaris says the default
	 * is 6, and sprintf on AIX defaults to 6
	 */
	if (max < 0)
		max = 6;

	ufvalue = abs_val (fvalue);

	if (fvalue < 0) {
		signvalue = '-';
	} else {
		if (flags & DP_F_PLUS) { /* Do a sign (+/i) */
			signvalue = '+';
		} else {
			if (flags & DP_F_SPACE)
				signvalue = ' ';
		}
	}

	/* 
	 * Sorry, we only support 16 digits past the decimal because of our 
	 * conversion method
	 */
	if (max > 16)
		max = 16;

	/* We "cheat" by converting the fractional part to integer by
	 * multiplying by a factor of 10
	 */

	temp = ufvalue;
	my_modf(temp, &intpart);

	fracpart = ROUND((POW10(max)) * (ufvalue - intpart));
	
	if (fracpart >= POW10(max)) {
		intpart++;
		fracpart -= POW10(max);
	}

	/* Convert integer part */
	do {
		temp = intpart*0.1;
		my_modf(temp, &intpart);
		idx = (int) ((temp -intpart +0.05)* 10.0);
		/* idx = (int) (((double)(temp*0.1) -intpart +0.05) *10.0); */
		/* printf ("%llf, %f, %x\n", temp, intpart, idx); */
		iconvert[iplace++] =
			(caps? "0123456789ABCDEF":"0123456789abcdef")[idx];
	} while (intpart && (iplace < 311));
	if (iplace == 311) iplace--;
	iconvert[iplace] = 0;

	/* Convert fractional part */
	if (fracpart)
	{
		do {
			temp = fracpart*0.1;
			my_modf(temp, &fracpart);
			idx = (int) ((temp -fracpart +0.05)* 10.0);
			fconvert[fplace++] =
			(caps? "0123456789ABCDEF":"0123456789abcdef")[idx];
		} while(fracpart && (fplace < 311));
		if (fplace == 311) fplace--;
	}
	fconvert[fplace] = 0;
  
	/* -1 for decimal point, another -1 if we are printing a sign */
	padlen = min - iplace - max - 1 - ((signvalue) ? 1 : 0); 
	zpadlen = max - fplace;
	if (zpadlen < 0) zpadlen = 0;
	if (padlen < 0) 
		padlen = 0;
	if (flags & DP_F_MINUS) 
		padlen = -padlen; /* Left Justifty */
	
	if ((flags & DP_F_ZERO) && (padlen > 0)) {
		if (signvalue) {
			DOPR_OUTCH(buffer, *currlen, maxlen, signvalue);
			--padlen;
			signvalue = 0;
		}
		while (padlen > 0) {
			DOPR_OUTCH(buffer, *currlen, maxlen, '0');
			--padlen;
		}
	}
	while (padlen > 0) {
		DOPR_OUTCH(buffer, *currlen, maxlen, ' ');
		--padlen;
	}
	if (signvalue) 
		DOPR_OUTCH(buffer, *currlen, maxlen, signvalue);
	
	while (iplace > 0) {
		--iplace;
		DOPR_OUTCH(buffer, *currlen, maxlen, iconvert[iplace]);
	}

	/*
	 * Decimal point.  This should probably use locale to find the correct
	 * char to print out.
	 */
	if (max > 0) {
		DOPR_OUTCH(buffer, *currlen, maxlen, '.');
		
		while (zpadlen > 0) {
			DOPR_OUTCH(buffer, *currlen, maxlen, '0');
			--zpadlen;
		}

		while (fplace > 0) {
			--fplace;
			DOPR_OUTCH(buffer, *currlen, maxlen, fconvert[fplace]);
		}
	}

	while (padlen < 0) {
		DOPR_OUTCH(buffer, *currlen, maxlen, ' ');
		++padlen;
	}
	return 0;
}

// safe print buffers from BSD these are correctly nulled out.
int va_snprintf (char *buffer, size_t buffersize, const char *format, ...)
{
    va_list args;
    int		result;

    va_start (args, format);
    result = va_vsnprintf (buffer, buffersize, format, args);
    va_end (args);

    return result;
}

int va_vsnprintf (char *buffer, size_t buffersize, const char *format, va_list args)
{
	return dopr(buffer, buffersize, format, args);
}
#endif

[qbism]

I have been getting by on this from Joequake

Code: Select all

void Q_strncpyz (char *dest, char *src, size_t size)
{
	strncpy (dest, src, size - 1);
	dest[size-1] = 0;
}

void Q_snprintfz (char *dest, size_t size, char *fmt, ...)
{
	va_list		argptr;

	va_start (argptr, fmt);
	vsnprintf (dest, size, fmt, argptr);
	va_end (argptr);

	dest[size-1] = 0;
}

[revelator]

yep easiest way to get around it mine might have its uses though as it can be expanded.

[Knightmare]

There's also Q_strncatz from QFusion and Quake2Evolved:

Code: Select all

void Q_strncatz (char *dst, const char *src, int dstSize)
{
	if (!dst)
		Com_Error(ERR_FATAL, "Q_strncatz: NULL dst");

	if (!src)
		Com_Error(ERR_FATAL, "Q_strncatz: NULL src");

	if (dstSize < 1)
		Com_Error(ERR_FATAL, "Q_strncatz: dstSize < 1");

	while (--dstSize && *dst)
		dst++;

	if (dstSize > 0){
		while (--dstSize && *src)
			*dst++ = *src++;

		*dst = 0;
	}
}

These Q_str* functions likely originated from the Q3 source, BTW.

But I found I already had these in my q_shared.c after Q2's standard Com_sprintf (already fixed to use _vsnprintf), and wasn't yet using them:

Code: Select all

void Com_strcpy (char *dest, int destSize, const char *src)
{
	if (!dest) {
		Com_Printf ("Com_strcpy: NULL dst\n");
		return;
	}
	if (!src) {
		Com_Printf ("Com_strcpy: NULL src\n");
		return;
	}
	if (destSize < 1) {
		Com_Printf ("Com_strcpy: dstSize < 1\n");
		return;
	}

	strncpy(dest, src, destSize-1);
	dest[destSize-1] = 0;
}


void Com_strcat (char *dest, int destSize, const char *src)
{
	if (!dest) {
		Com_Printf ("Com_strcat: NULL dst\n");
		return;
	}
	if (!src) {
		Com_Printf ("Com_strcat: NULL src\n");
		return;
	}
	if (destSize < 1) {
		Com_Printf ("Com_strcat: dstSize < 1\n");
		return;
	}

	while (--destSize && *dest)
		dest++;

	if (destSize > 0) {
		while (--destSize && *src)
			*dest++ = *src++;

		*dest = 0;
	}
}

[revelator]

tbh it shouldnt be nessesary to use our own library replacements for anything but strlcpy and strlcat if using msvc > 2003 since those allready have str safe functions.
it would require a few defines to check for recent msvc but in the end it would make the source smaller.

_vsnprintf is an edge case though as the msvc and mingw64 ones returns bogus output (i know from git and several other sources that its broken).
_snprintf is also affected.
the bsd function i posted above returns the correct output and is used in the beforementioned sources.
strlcpy and strlcat might not even be nessesary either with strncpy_s and strncat_s.

[revelator]

Let me explain as good as i can why _vsnprintf and _snprintf are broken on windows.
on error _vsnprintf returns -1 where it should return the number of bytes it could have written to dest.
same goes for _snprintf.
Mingw has a vsnprintf that returns the correct value (notice the lack of _ in front of the name).
Mingw also has a snprintf which returns the correct value.
Msvc lacks those so to get the correct output we need to roll our own versions as return -1 triggers a fault (yep i actually tested this).
Mingw also has the _vsnprintf function with the same bug as msvc so do not use this version instead use the one without the _.

I can provide test code so you can see for yourself

[Knightmare]

Does the return -1 trigger a fault in _vsnprintf, or in the code that calls it, if that code expects an unsigned value for the return or something? TTimo used this in the released Q3 source (abstracted as Q_vsnprintf), and never used the return value.
This is the only place in my source that uses the value returned:

Code: Select all

void Com_sprintf (char *dest, int size, char *fmt, ...)
{
	int		len;
	va_list		argptr;
	char	bigbuffer[0x10000];

	va_start (argptr, fmt);
	len = Q_vsnprintf (bigbuffer, sizeof(bigbuffer), fmt, argptr);
	va_end (argptr);
	if (len >= size)
		Com_Printf ("Com_sprintf: overflow of %i in %i\n", len, size);
	strncpy (dest, bigbuffer, size-1);
	dest[size-1] = 0;
}

If that would indeed be a problem, can I just safely ignore the retval?

[mh]

I agree with Knightmare here; I'm having a hard time seeing how a return value could generate a fault in the function that returns the value. Checking my MSDN, I see that _vsnprintf is specified to return one of three things:

• The number of characters written,
• -1 if the number of characters exceeds the count param,
• An unspecified negative value if an error occurs.

The only thing that seems wrong here is the -1 return; C99 (and presumably previous standards, as well as C11) specifies that the number of characters that would have been written is returned, which can then be checked against the buffer size and an appropriate response chosen. But the only possible way I can see -1 triggering a fault is if you're not checking your return for negative values (which are specified by the standard; e.g. http://netbsd.gw.com/cgi-bin/man-cgi?vsnprintf "If an output error was encountered, these functions shall return a negative value"), casting it to unsigned (so -1 becomes 0xffffffff) and trying to alloc a new buffer of that size for a retry. In that case you're probably not checking malloc return values either, and probably not running in a debugger to make things even worse.

[revelator]

this was not found by me but by the texlive and git creators.
if the value returned was actually just a negative number it wouldnt have mattered but it translates to 'bogus value' in msvc syntax and triggers a fault because the functions need to know the size it could have allocated even if the string is empty.

part of the patch with comments from the git folks.

+# Define SNPRINTF_RETURNS_BOGUS if your are on a system which snprintf()
+# returns -1 instead of number of characters which would have been written
+# to the final string if enough space had been available.
+#
+# Define VSNPRINTF_RETURNS_BOGUS if your are on a system which vsnprintf()
+# returns -1 instead of number of characters which would have been written
+# to the final string if enough space had been available.
+#

the above applies to msvc/mingw64/mac and a few other obscure systems where it returns -1 the edge case here is mingw which has a vsnprintf without the _ in front which works.
i tried with both just for fun using C::B + mingw and _vsnprintf crashes kernel32.dll while vsnprintf works as expected. mingw64 does not have the vsnprintf function from mingw so it also goes bad

[revelator]

Test code.

Code: Select all

#include <stddef.h>
#include <stdarg.h>
#include <stdio.h>

#include "config.h"


#ifdef _RWSTD_NO_VSNPRINTF
#  if !defined (_MSC_VER)

extern "C" int vsnprintf (char*, size_t, const char*, va_list);

#  else   // if defined (_MSC_VER)

extern "C" int _vsnprintf (char*, size_t, const char*, va_list);

#    define vsnprintf _vsnprintf

#  endif   // MSVC

#endif   // _RWSTD_NO_VSNPRINTF


int my_snprintf (char *buf, size_t size, const char *fmat, ...)
{
    va_list va;
    va_start (va, fmat);

    const int ret = vsnprintf (buf, size, fmat, va);

    va_end (va);

    return ret;
}


int test_vsnprintf (int success)
{
    char buf [] = "0123456789";

    const char data [] = "abcdef";

    const size_t bufsize  = success ? sizeof buf : 4U;
    const size_t datasize = sizeof data;
    
    const int ret = my_snprintf (buf, bufsize, "%s", data);

    // 7.19.6.12, p3 of C99:
    //
    // The vsnprintf function returns the number of characters that would
    // have been written had n been sufficiently large, not counting the
    // terminating null character, or a negative value if an encoding error
    // occurred. Thus, the null-terminated output has been completely
    // written if and only if the returned value is nonnegative and less
    // than n.

    if (success) {
        if (datasize - 1 == ret) {
            // expected behavior, no output
            return 0;
        }
        else if (datasize == ret) {
            printf ("#define _RWSTD_NO_VSNPRINTF_NUL "
                    "/* return value includes NUL on success */\n");
            return -1;
        }
        else if (ret < 0) {
            printf ("#define _RWSTD_NO_VSNPRINTF_POS "
                    "/* return value negative on success */\n");
            return 1;
        }
        else {
            printf ("#define _RWSTD_NO_VSNPRINTF_VALID "
                    "/* return value bogus on success */\n");
            return 1;
        }
    }
    else {
        if (datasize - 1 == ret) {
            // expected behavior, no output
            return 0;
        }
        else if (ret < 0) {
            // HP-UX and Windows are known to return -1 on buffer overflow
            printf ("#define _RWSTD_NO_VSNPRINTF_VFLOW_VALID "
                    "/* return value negative on overflow */\n");
            return 1;
        }
        else if (3 == ret) {
            // IRIX and Tru64 UNIX are known to return the number
            // of bytes actually written which is useless for
            // determing the size of the buffer
            printf ("#define _RWSTD_NO_VSNPRINTF_VFLOW_VALID "
                    "/* returns the number of non-NUL bytes written */\n");
            return 1;
        }
        else if (4 == ret) {
            printf ("#define _RWSTD_NO_VSNPRINTF_VFLOW_VALID "
                    "/* returns the number of bytes written */\n");
            return 1;
        }
        else {
            printf ("#define _RWSTD_NO_VSNPRINTF_VFLOW_VALID "
                    "/* return value bogus on overflow */\n");
            return 1;
        }
    }
}


int main ()
{
    test_vsnprintf (0);
    test_vsnprintf (1);

    return 0;
}

win seems to return -1 on buffer overflow so one could make sure the allocated buffers are large enough (or get told the hard way with a crash with no info other than the debugger stating that you had a buffer overflow in your code then kills the process so its real nasty to debug). the bsd version also crashes on this but it tells you where so happy face ID's later code like the wrappers in Q3 also tells you a bit more about where things go wrong. We could expand these to also tell us in what function the bad bad things happen like this version.

Code: Select all

int va_snprintf (char *function, char *buffer, size_t buffersize, const char *format, ...)
{
    va_list args;
    int		result;

    va_start (args, format);
    result = va_vsnprintf (function, buffer, buffersize, format, args);
    va_end (args);

    return result;
}

int va_vsnprintf (char *function, char *buffer, size_t buffersize, const char *format, va_list args)
{
    size_t result;

    result = _vsnprintf (buffer, buffersize, format, args);

    if (result < 0 || result >= buffersize)
    {
        static qboolean inside;

        // Beware recursion here
        if (!inside)
        {
            inside = true;
            Con_SafePrintf ("%s: excessive string length, max = %d\n", function, buffersize);
        }
        inside = false;
        buffer[buffersize - 1] = '\0'; 
        return -1;
    }
    return (int) result;
}

i use the above in realm but it originated from bengt jardrups code.

[Knightmare]

You still didn't answer my question of whether calls to _vsnprintf that don't capture the return value are unsafe, or if I could safely handle return values of -1, like this:

Code: Select all

void Com_sprintf (char *dest, int size, char *fmt, ...)
{
	int		len;
	va_list		argptr;
	char	bigbuffer[0x10000];

	va_start (argptr, fmt);
	len = Q_vsnprintf (bigbuffer, sizeof(bigbuffer), fmt, argptr);
	va_end (argptr);
	if (len < 0 || len >= size)
		Com_Printf ("Com_sprintf: overflow in dest buffer of size %i\n", size);
	strncpy (dest, bigbuffer, size-1);
	dest[size-1] = 0;
}