Re: [PATCHES] [COMMITTERS] pgsql: Allow psql multi-line column values to align
Martijn van Oosterhout writes: > Patch attached. Passes -pedantic on gcc 3.3.5 Applied with some cosmetic cleanups and further fixes for 64-bit problems. regards, tom lane ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] [COMMITTERS] pgsql: Allow psql multi-line column values to align
On Fri, Feb 10, 2006 at 02:13:26PM -0500, Tom Lane wrote:
> For the most part we say "char" where we can and "unsigned char" only
> where it really matters, which is mostly inside code that's
> encoding-aware anyway.
Well, I've done this and avoided changing any public interfaces. ie the
libpq interface remains unsigned, as does the psql formatting code, but
the printTable stuff only is for the parts that actually do
formatting...
Patch attached. Passes -pedantic on gcc 3.3.5
Have a nice day,
--
Martijn van Oosterhout http://svana.org/kleptog/
> Patent. n. Genius is 5% inspiration and 95% perspiration. A patent is a
> tool for doing 5% of the work and then sitting around waiting for someone
> else to do the other 95% so you can sue them.
Index: src/bin/psql/mbprint.c
===
RCS file: /projects/cvsroot/pgsql/src/bin/psql/mbprint.c,v
retrieving revision 1.19
diff -u -r1.19 mbprint.c
--- src/bin/psql/mbprint.c 10 Feb 2006 00:39:04 - 1.19
+++ src/bin/psql/mbprint.c 10 Feb 2006 21:46:49 -
@@ -158,11 +158,11 @@
{
int chlen, chwidth;
- chlen = PQmblen(pwcs, encoding);
+ chlen = PQmblen( (char*)pwcs, encoding);
if (chlen > len)
break; /* Invalid string */
- chwidth = PQdsplen(pwcs, encoding);
+ chwidth = PQdsplen( (char*)pwcs, encoding);
if (chwidth > 0)
width += chwidth;
@@ -191,10 +191,10 @@
for (; *pwcs && len > 0; pwcs += chlen)
{
- chlen = PQmblen(pwcs, encoding);
+ chlen = PQmblen( (char*)pwcs, encoding);
if (len < (size_t)chlen)
break;
- w = PQdsplen(pwcs, encoding);
+ w = PQdsplen( (char*)pwcs, encoding);
if (chlen == 1) /* ASCII char */
{
@@ -257,14 +257,14 @@
chlen = 0;
int linewidth = 0;
- char *ptr = lines->ptr; /* Pointer to data area */
+ unsigned char *ptr = lines->ptr; /* Pointer to data area */
for (; *pwcs && len > 0; pwcs += chlen)
{
- chlen = PQmblen(pwcs,encoding);
+ chlen = PQmblen( (char*)pwcs,encoding);
if (len < (size_t)chlen)
break;
- w = PQdsplen(pwcs,encoding);
+ w = PQdsplen( (char*)pwcs,encoding);
if (chlen == 1) /* single byte char char */
{
@@ -282,13 +282,13 @@
}
else if (*pwcs == '\r') /* Linefeed */
{
- strcpy(ptr, "\\r");
+ strcpy( (char*)ptr, "\\r");
linewidth += 2;
ptr += 2;
}
else if (w <= 0) /* Other control char */
{
- sprintf(ptr, "\\x%02X", *pwcs);
+ sprintf( (char*)ptr, "\\x%02X", *pwcs);
linewidth += 4;
ptr += 4;
}
@@ -301,13 +301,13 @@
else if (w <= 0) /* Non-ascii control char */
{
if (encoding == PG_UTF8)
- sprintf(ptr, "\\u%04X", utf2ucs(pwcs));
+ sprintf( (char*)ptr, "\\u%04X", utf2ucs(pwcs));
else
/* This case cannot happen in the current
* code because only UTF-8 signals multibyte
* control characters. But we may need to
* support it at some stage */
- sprintf(ptr, "\\u");
+ sprintf( (char*)ptr, "\\u");
ptr += 6;
linewidth += 6;
Index: src/bin/psql/print.c
===
RCS file: /projects/cvsroot/pgsql/src/bin/psql/print.c,v
retrieving revision 1.81
diff -u -r1.81 print.c
--- src/bin/psql/print.c10 Feb 2006 15:48:05 - 1.81
+++ src/bin/psql/print.c10 Feb 2006 21:46:49 -
@@ -357,8 +357,8 @@
{
unsigned int col_count = 0;
unsigned int cell_count = 0;
- unsigned int i,
- tmp;
+ unsigned int i;
+ int tmp;
unsigned int *widths,
total_w;
unsigned int *heights;
@@ -588,7 +588,7 @@
* this_line->width <
strlen(this_ptr) and we
Re: [PATCHES] [COMMITTERS] pgsql: Allow psql multi-line column values to align
Martijn van Oosterhout writes: > Does PostgreSQL have a policy on the signedness of strings? For the most part we say "char" where we can and "unsigned char" only where it really matters, which is mostly inside code that's encoding-aware anyway. It was only fairly recently that we cleaned the code up to avoid signedness warnings, but now that that's done I don't want to backtrack on it. What I'd suggest is taking a close look at the set of functions you have and trying to identify a layer that should be "unsigned char" versus upper layers that can just say "char". regards, tom lane ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] [COMMITTERS] pgsql: Allow psql multi-line column values to align
On Fri, Feb 10, 2006 at 06:16:07PM +0100, Martijn van Oosterhout wrote: > Thanks for the tip. I'm currently merging CVS with my version and > getting a lot of conflicts (whitespace variations). > > It's fairly simple changes AFAICS. Just need to fix the declarations of > a few variables. Does PostgreSQL have a policy on the signedness of strings? When I was making the patch I generally used "unsigned char" to try to emphasise that these are not characters, they are bytes of an encoding. But this doesn't play well with existing functions like PQdsplen and PQmblen. At this point I'm thinking of dropping the "unsigned" everywhere and just being really careful of comparisons. OTOH, it's really those two functions being inconsistant because the underlying functions do use unsigned. Thoughts? Have a nice day, -- Martijn van Oosterhout http://svana.org/kleptog/ > Patent. n. Genius is 5% inspiration and 95% perspiration. A patent is a > tool for doing 5% of the work and then sitting around waiting for someone > else to do the other 95% so you can sue them. ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] [COMMITTERS] pgsql: Allow psql multi-line column values to align
On Fri, Feb 10, 2006 at 12:09:13PM -0500, Bruce Momjian wrote: > Tom Lane wrote: > > Bruce Momjian writes: > > > Martijn van Oosterhout wrote: > > >> I'll try to address these warnings (unless someone beats me to it). > > > > > I am looking at it now but I can't find the compiler flag to get those > > > warnings. I am gcc 2.95.3. > > > > You'd need a newer compiler. > > gcc -pedantic is showing it to me. Thanks for the tip. I'm currently merging CVS with my version and getting a lot of conflicts (whitespace variations). It's fairly simple changes AFAICS. Just need to fix the declarations of a few variables. -- Martijn van Oosterhout http://svana.org/kleptog/ > Patent. n. Genius is 5% inspiration and 95% perspiration. A patent is a > tool for doing 5% of the work and then sitting around waiting for someone > else to do the other 95% so you can sue them. signature.asc Description: Digital signature
Re: [PATCHES] [COMMITTERS] pgsql: Allow psql multi-line column values to align
Bruce Momjian writes: > Martijn van Oosterhout wrote: >> I'll try to address these warnings (unless someone beats me to it). > I am looking at it now but I can't find the compiler flag to get those > warnings. I am gcc 2.95.3. You'd need a newer compiler. regards, tom lane ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] [COMMITTERS] pgsql: Allow psql multi-line column values to align
On Fri, Feb 10, 2006 at 11:02:41AM -0500, Tom Lane wrote: > Bruce Momjian writes: > > OK, I have applied your patch to psql and Teodor has adjusted the > > tsearch2 expected results. I can't seem to run pgcrypto without getting > > PRNG errors, so I expect my SSL is too old. Would you send me your > > pgcrypto/regression.diff? Thanks. > > I fixed that already. Please see about fixing the compiler warnings you > introduced. On a 64-bit machine there are even more, and the int vs > size_t ones are definitely portability problems. Ouch. I submitted this patch so long ago I figured it'd gotten lost. I was actually considering preparing a new version against HEAD. I'll try to address these warnings (unless someone beats me to it). -- Martijn van Oosterhout http://svana.org/kleptog/ > Patent. n. Genius is 5% inspiration and 95% perspiration. A patent is a > tool for doing 5% of the work and then sitting around waiting for someone > else to do the other 95% so you can sue them. signature.asc Description: Digital signature
Re: [PATCHES] [COMMITTERS] pgsql: Allow psql multi-line column values to align
Bruce Momjian writes: > OK, I have applied your patch to psql and Teodor has adjusted the > tsearch2 expected results. I can't seem to run pgcrypto without getting > PRNG errors, so I expect my SSL is too old. Would you send me your > pgcrypto/regression.diff? Thanks. I fixed that already. Please see about fixing the compiler warnings you introduced. On a 64-bit machine there are even more, and the int vs size_t ones are definitely portability problems. regards, tom lane print.c: In function 'print_aligned_text': print.c:408: warning: pointer targets in passing argument 4 of 'pg_wcssize' differ in signedness print.c:428: warning: pointer targets in passing argument 4 of 'pg_wcssize' differ in signedness print.c:479: warning: pointer targets in passing argument 4 of 'pg_wcssize' differ in signedness print.c:588: warning: pointer targets in passing argument 1 of 'format_numeric_locale' differ in signedness print.c:589: warning: field width should have type 'int', but argument 3 has type 'size_t' print.c: In function 'print_aligned_vertical': print.c:690: warning: pointer targets in passing argument 4 of 'pg_wcssize' differ in signedness print.c:714: warning: pointer targets in passing argument 4 of 'pg_wcssize' differ in signedness print.c:823: warning: pointer targets in passing argument 1 of 'format_numeric_locale' differ in signedness print.c:827: warning: field width should have type 'int', but argument 4 has type 'size_t' print.c: In function 'printQuery': print.c:1753: warning: pointer targets in passing argument 1 of 'mbvalidate' differ in signedness print.c:1753: warning: pointer targets in assignment differ in signedness print.c:1764: warning: pointer targets in passing argument 1 of 'mbvalidate' differ in signedness print.c:1764: warning: pointer targets in assignment differ in signedness mbprint.c: In function 'pg_wcswidth': mbprint.c:161: warning: pointer targets in passing argument 1 of 'PQmblen' differ in signedness mbprint.c:165: warning: pointer targets in passing argument 1 of 'PQdsplen' differ in signedness mbprint.c: In function 'pg_wcssize': mbprint.c:194: warning: pointer targets in passing argument 1 of 'PQmblen' differ in signedness mbprint.c:197: warning: pointer targets in passing argument 1 of 'PQdsplen' differ in signedness mbprint.c: In function 'pg_wcsformat': mbprint.c:260: warning: pointer targets in initialization differ in signedness mbprint.c:264: warning: pointer targets in passing argument 1 of 'PQmblen' differ in signedness mbprint.c:267: warning: pointer targets in passing argument 1 of 'PQdsplen' differ in signedness mbprint.c:281: warning: pointer targets in assignment differ in signedness print.c: In function 'print_aligned_text': print.c:408: warning: pointer targets in passing argument 4 of 'pg_wcssize' differ in signedness print.c:428: warning: pointer targets in passing argument 4 of 'pg_wcssize' differ in signedness print.c:479: warning: pointer targets in passing argument 4 of 'pg_wcssize' differ in signedness print.c:588: warning: pointer targets in passing argument 1 of 'format_numeric_locale' differ in signedness print.c:589: warning: field width should have type 'int', but argument 3 has type 'size_t' print.c: In function 'print_aligned_vertical': print.c:690: warning: pointer targets in passing argument 4 of 'pg_wcssize' differ in signedness print.c:714: warning: pointer targets in passing argument 4 of 'pg_wcssize' differ in signedness print.c:823: warning: pointer targets in passing argument 1 of 'format_numeric_locale' differ in signedness print.c:827: warning: field width should have type 'int', but argument 4 has type 'size_t' print.c: In function 'printQuery': print.c:1753: warning: pointer targets in passing argument 1 of 'mbvalidate' differ in signedness print.c:1753: warning: pointer targets in assignment differ in signedness print.c:1764: warning: pointer targets in passing argument 1 of 'mbvalidate' differ in signedness print.c:1764: warning: pointer targets in assignment differ in signedness mbprint.c: In function 'pg_wcswidth': mbprint.c:161: warning: pointer targets in passing argument 1 of 'PQmblen' differ in signedness mbprint.c:165: warning: pointer targets in passing argument 1 of 'PQdsplen' differ in signedness mbprint.c: In function 'pg_wcssize': mbprint.c:194: warning: pointer targets in passing argument 1 of 'PQmblen' differ in signedness mbprint.c:197: warning: pointer targets in passing argument 1 of 'PQdsplen' differ in signedness mbprint.c: In function 'pg_wcsformat': mbprint.c:260: warning: pointer targets in initialization differ in signedness mbprint.c:264: warning: pointer targets in passing argument 1 of 'PQmblen' differ in signedness mbprint.c:267: warning: pointer targets in passing argument 1 of 'PQdsplen' differ in signedness mbprint.c:281: warning: pointer targets in assignment differ in signedness ---(end of broadcast)
Re: [PATCHES] [COMMITTERS] pgsql: Allow psql multi-line column values to align
OK, I have applied your patch to psql and Teodor has adjusted the tsearch2 expected results. I can't seem to run pgcrypto without getting PRNG errors, so I expect my SSL is too old. Would you send me your pgcrypto/regression.diff? Thanks. --- Kris Jurka wrote: > > > On Thu, 9 Feb 2006, Bruce Momjian wrote: > > > Log Message: > > --- > > Allow psql multi-line column values to align in the proper columns > > > > There is a problem with this on AIX. > > http://pgbuildfarm.org/cgi-bin/show_log.pl?nm=asp&dt=2006-02-10%2006:23:00 > > For tables that have no columns AIX thinks it has an out of memory error. > src/bin/psql/print.c:pg_local_calloc bails out if calloc returns NULL. I > believe AIX is returning NULL for calloc with a count of zero. Efforts > are made not to call pg_local_calloc with a count of zero, but one place > is missed, the attached patch fixes that. > > Additionally there are a whole lot of of signedness warnings introduced > which I've attached. > > Kris Jurka Content-Description: [ Attachment, skipping... ] Content-Description: [ Attachment, skipping... ] > > ---(end of broadcast)--- > TIP 1: if posting/reading through Usenet, please send an appropriate >subscribe-nomail command to [EMAIL PROTECTED] so that your >message can get through to the mailing list cleanly -- Bruce Momjian| http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
