This is the mail archive of the
newlib@sources.redhat.com
mailing list for the newlib project.
Re: %S %C vfprintf contribution
Hello Jeff!
J. Johnston wrote:
Artem,
A number of comments/issues:
1. The tests for buf size vs MB_LEN_MAX shouldn't be needed.
The lowest value is 40 and I don't think I want to see a character
set that requires over 40 bytes to encode a single character. :)
I don't think it is an error - it is just paranoid check. Theoretically,
MB_LEN_MAX may be > 40 (I think only about variable rang - not about
real multibyte characters). This is preprocessor check an it doesn't
make much overhead. :-) Please, remove this if you consider necessary.
2. Your 'S' code ends up falling into the old strlen check for 's'. You
should prevent this by using an else clause.
Hmm, I think you missed about 'break' at line 728...
Your check for cp is
NULL should occur at the top just after setting cp. Your S code can
then be an else if and the old 's' code can be in an else clause.
If cp is NULL we consider this as if it was called with %s and set it to
"(null)". (This means, treat calls vfprintf("%S", NULL) = vfprintf("%s",
NULL)). In both cases "(null)" string will appead in file stream. Ther
is a check for cp != NULL at line 678. Is this an error?
3. Your freeing of the widechar string buffer is not in the right spot.
There is an error case that follows allocating the buffer so you
won't
free it.
Yes, you are right, I've missed this.
You should also use a separate ptr other than cp for the
malloc'd storage so there is no doubt you are freeing the right
thing
at the end.
We are setting WIDECHARSTR flag only on line 719 after malloc. I think
it is no need to introduce another pointer.
-- Jeff J.
Artem B. Bityuckiy wrote:
Artem B. Bityuckiy wrote:
Hello.
Nicholas Wourms wrote:
> Artem B. Bityuckiy wrote:
>
>> Hello.
>>
>> Here is the patch that makes vfprintf (and hence, all other
>> vfprintf-based Xprintf functions) understand ISO C 90 %S (same as
%ls)
>> and %C (same as %lc) format placeholders. Please, review it and give
>> us know if something is wrong.
>>
>> This is tested a little bit- it works and it seems should work with
>> any locale if wcrtomb and wcsrtomb functions work.
>>
>
> I believe the preferred patch type is unidiff or `diff -up`.
OK, here is the patch to vfprintf.c produced by diff -up.
>
> Cheers,
> Nicholas
>
Initially, we've added these changes to Newlib-1.11.0. They was
tested with Newlib-1.11.0 too. But then I've copy changes to Newlib
from CVS. vfprintf.c from CVS differs from Newlib-1.11.0's
vfprintf.c. I didn't test (even compile) Newlib from CVS with these
patch, but it must work.
Thanks.
I've attached new diff -uN file.
--
Best regards,
Artem B. Bityuckiy
SoftMine Corporation, Software Engineer
Tel.: +7(812)329-67-44, +7(812)329-67-45
Mobile: +79112449030
E-mail: abitytsky@softminecorp.com
Web: www.softminecorp.com
--- vfprintf_cvs.c 2003-10-30 13:31:41.000000000 +0300
+++ vfprintf_smc_cvs.c 2003-11-01 12:16:10.000000000 +0300
@@ -162,6 +162,7 @@
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
+#include <limits.h>
#include <reent.h>
#ifdef _HAVE_STDC
@@ -237,7 +238,12 @@
#include <math.h>
#include "floatio.h"
-#define BUF (MAXEXP+MAXFRACT+1) /* + decimal point */
+#if ((MAXEXP+MAXFRACT+1) > MB_LEN_MAX)
+# define BUF (MAXEXP+MAXFRACT+1) /* + decimal point */
+#else
+# define BUF MB_LEN_MAX
+#endif
+
#define DEFPREC 6
static char *cvt _PARAMS((struct _reent *, double, int, int, char *, int *, int, int *));
@@ -245,7 +251,11 @@
#else /* no FLOATING_POINT */
-#define BUF 40
+#if (MB_LEN_MAX < 40)
+# define BUF 40
+#else
+# define BUF MB_LEN_MAX
+#endif
#endif /* FLOATING_POINT */
@@ -269,6 +279,7 @@
#define SHORTINT 0x040 /* short integer */
#define ZEROPAD 0x080 /* zero (as opposed to blank) pad */
#define FPT 0x100 /* Floating point number */
+#define WIDECHARSTR 0x200 /* Widechar string */
int
_DEFUN (VFPRINTF, (fp, fmt0, ap),
@@ -520,7 +531,23 @@
flags |= QUADINT;
goto rflag;
case 'c':
- *(cp = buf) = va_arg(ap, int);
+ case 'C':
+ cp = buf;
+ if (*fmt == 'C' || (flags & LONGINT))
+ {
+ mbstate_t ps;
+
+ memset((void *)&ps, '\0', sizeof(mbstate_t));
+ if ((size = (int)wcrtomb(cp,
+ (wchar_t)va_arg(ap, int),
+ &ps)) == -1)
+ goto error;
+ }
+ else
+ {
+ *cp = (char)va_arg(ap, int);
+ size = 1;
+ }
size = 1;
sign = '\0';
break;
@@ -644,8 +671,69 @@
ch = 'x';
goto nosign;
case 's':
- if ((cp = va_arg(ap, char *)) == NULL)
- cp = "(null)";
+ case 'S':
+ sign = '\0';
+ cp = va_arg(ap, char *);
+
+ if (cp && (ch == 'S' || (flags & LONGINT))) {
+ mbstate_t ps;
+ _CONST wchar_t *wcp;
+
+ wcp = (_CONST wchar_t *)cp;
+ size = m = 0;
+ memset((void *)&ps, '\0', sizeof(mbstate_t));
+
+ /*
+ * Count number of bytes needed to store multibyte
+ * string that will be produced from widechar
+ * string.
+ */
+ if (prec >= 0) {
+ while (1) {
+ if (wcp[m] == L'\0')
+ break;
+ if ((n = (int)wcrtomb(buf,
+ wcp[m], &ps)) == -1)
+ goto error;
+ if (n + size > prec)
+ break;
+ m += 1;
+ size += n;
+ if (size == prec)
+ break;
+ }
+ }
+ else {
+ if ((size = (int)wcsrtombs(NULL, &wcp,
+ 0, &ps)) == -1)
+ goto error;
+ wcp = (_CONST wchar_t *)cp;
+ }
+
+ if (size == 0)
+ break;
+
+ if ((cp = (char *)malloc(size + 1)) == NULL)
+ goto error;
+
+ flags |= WIDECHARSTR;
+
+ /*
+ * Convert widechar string to multibyte string.
+ */
+ memset((void *)&ps, '\0', sizeof(mbstate_t));
+ if (wcsrtombs(cp, &wcp, size, &ps) != size)
+ {
+ free(cp);
+ goto error;
+ }
+ cp[size] = '\0';
+ break;
+ }
+
+ if (cp == NULL)
+ cp = "(null)";
+
if (prec >= 0) {
/*
* can't use strlen; can only look for the
@@ -662,7 +750,6 @@
size = prec;
} else
size = strlen(cp);
- sign = '\0';
break;
case 'U':
flags |= LONGINT;
@@ -846,6 +933,9 @@
ret += width > realsz ? width : realsz;
FLUSH(); /* copy out the I/O vectors */
+
+ if (flags & WIDECHARSTR)
+ free(cp);
}
done:
FLUSH();