Thread: patch: contrib/pgcrypto sanity
The KAME files md5.* and sha1.* have the following changelog entry: ---------------------------- revision 1.2 date: 2000/12/04 01:20:38; author: tgl; state: Exp; lines: +18 -18 Eliminate some of the more blatant platform-dependencies ... it builds here now, anyway ... ---------------------------- Which basically changes u_int*_t -> uint*_t, so now it does not compile neither under Debian 2.2 nor under NetBSD 1.5 which is platform independent¸ all right. Also it replaces $KAME$ with $Id$ which is Bad Thing. PostgreSQL Id should be added as a separate line so the file history could be seen. So here is patch: * changes uint*_t -> uint*. I guess that was the original intention * adds uint64 type to include/c.h because its needed [somebody should check if I did it right] * adds back KAME Id, because KAME is the master repository * removes stupid c++ comments in pgcrypto.c * removes <sys/types.h> from the code, its not needed -- marko
Attachment
Marko Kreen <marko@l-t.ee> writes:
> date: 2000/12/04 01:20:38; author: tgl; state: Exp; lines: +18 -18
> Eliminate some of the more blatant platform-dependencies ... it
> builds here now, anyway ...
> Which basically changes u_int*_t -> uint*_t, so now it does not
> compile neither under Debian 2.2 nor under NetBSD 1.5 which
> is platform independent� all right.
Well, that's annoying. I guess those platforms are out of step with the
C99 standard, which specifies uint*_t not u_int*_t (cf. C99 7.4.1.1).
I agree with your solution of switching to Postgres-supplied typenames.
> Also it replaces $KAME$ with $Id$ which is Bad Thing. PostgreSQL Id
> should be added as a separate line so the file history could be seen.
I didn't know what $KAME$ was, and took it for some national-language
variant of $Id$ that our CVS server didn't know about. Who/what is
KAME anyway?
> * adds back KAME Id, because KAME is the master repository
Is that a good idea? If we are deliberately deviating from whatever
"master repository" this is, I'm not sure that we should continue to
label the code with their revision ID.
regards, tom lane
On Sun, Jan 07, 2001 at 08:09:07PM -0500, Tom Lane wrote: > Marko Kreen <marko@l-t.ee> writes: > > date: 2000/12/04 01:20:38; author: tgl; state: Exp; lines: +18 -18 > > Eliminate some of the more blatant platform-dependencies ... it > > builds here now, anyway ... > > > Which basically changes u_int*_t -> uint*_t, so now it does not > > compile neither under Debian 2.2 nor under NetBSD 1.5 which > > is platform independent¸ all right. > > Well, that's annoying. I guess those platforms are out of step with the > C99 standard, which specifies uint*_t not u_int*_t (cf. C99 7.4.1.1). > I agree with your solution of switching to Postgres-supplied typenames. Well, actually they do. glibc in <stdint.h> and NetBSD in <sys/inttypes.h> which is a mess, all rigth. Problem is that postgres.h does not know about this. I guess that C99 forgot to specify _where_ they should be defined. But the point is, what are the standard types in PostgreSQL? C99 ones are not, because there would be lots of noise raised already. If you think that the C99 should be standard someday, then postgres.h should also provide them, because older platforms definitely do not provide them. > > Also it replaces $KAME$ with $Id$ which is Bad Thing. PostgreSQL Id > > should be added as a separate line so the file history could be seen. > > I didn't know what $KAME$ was, and took it for some national-language > variant of $Id$ that our CVS server didn't know about. Who/what is > KAME anyway? KAME is the project that provides IPv6 to all the free BSD's. (www.kame.net) > > * adds back KAME Id, because KAME is the master repository > > Is that a good idea? If we are deliberately deviating from whatever > "master repository" this is, I'm not sure that we should continue to > label the code with their revision ID. I think it is. I think they are more competent on maintaining crypto than PostgreSQL team (at least they devote more time on it). This makes tracking changes easier. This (special Id's) are widely used practice in BSD's, this makes clear whether a file is 'original' or 'imported'; and from where. -- marko
On Mon, Jan 08, 2001 at 04:06:09AM +0200, Marko Kreen wrote: > On Sun, Jan 07, 2001 at 08:09:07PM -0500, Tom Lane wrote: > > Marko Kreen <marko@l-t.ee> writes: > > > Which basically changes u_int*_t -> uint*_t, so now it does not > > > compile neither under Debian 2.2 nor under NetBSD 1.5 which > > > is platform independent¸ all right. > > > > Well, that's annoying. I guess those platforms are out of step with the > > C99 standard, which specifies uint*_t not u_int*_t (cf. C99 7.4.1.1). > > I agree with your solution of switching to Postgres-supplied typenames. > > Well, actually they do. glibc in <stdint.h> and NetBSD in > <sys/inttypes.h> which is a mess, all rigth. Problem is that > postgres.h does not know about this. I guess that C99 forgot > to specify _where_ they should be defined. Correction, they both have <inttypes.h> which probably is the right location for this. > > > * adds back KAME Id, because KAME is the master repository > > > > Is that a good idea? If we are deliberately deviating from whatever > > "master repository" this is, I'm not sure that we should continue to > > label the code with their revision ID. > > I think it is. I think they are more competent on maintaining > crypto than PostgreSQL team (at least they devote more time on > it). This makes tracking changes easier. This (special Id's) > are widely used practice in BSD's, this makes clear whether a > file is 'original' or 'imported'; and from where. The "master" only means that occasionally somebody (e.g. I) can check if their revision is increased and then can merge changes into our sources, if needed. Also _I_ know where I got these sources but this makes it easier for others too. -- marko
Marko Kreen writes:> On Mon, Jan 08, 2001 at 04:06:09AM +0200, Marko Kreen wrote:> > On Sun, Jan 07, 2001 at 08:09:07PM -0500,Tom Lane wrote:> > > Marko Kreen <marko@l-t.ee> writes:> > > > Which basically changes u_int*_t -> uint*_t, so nowit does> > > > not compile neither under Debian 2.2 nor under NetBSD 1.5> > > > which is platform independent¸ all right.>> > > > > Well, that's annoying. I guess those platforms are out of step> > > with the C99 standard, which specifiesuint*_t not u_int*_t> > > (cf. C99 7.4.1.1). I agree with your solution of switching to> > > Postgres-suppliedtypenames.> > > > Well, actually they do. glibc in <stdint.h> and NetBSD in> > <sys/inttypes.h> which isa mess, all rigth. Problem is that> > postgres.h does not know about this. I guess that C99 forgot to> > specify _where_they should be defined.> > Correction, they both have <inttypes.h> which probably is the right> location for this. <stdint.h> is adequate to pick up uint*_t. <inttypes.h> is defined to include <stdint.h>. Of course all this C99 stuff is new and existing implementations may have the typedefs in different files or not have them at all. -- Pete Forman -./\.- Disclaimer: This post is originated WesternGeco -./\.- by myself and does not represent pete.forman@westerngeco.com -./\.- opinion of Schlumberger, Baker http://www.crosswinds.net/~petef -./\.- Hughes or their divisions.
On Mon, Jan 08, 2001 at 10:03:25AM +0000, Pete Forman wrote:
> Marko Kreen writes:
> > On Mon, Jan 08, 2001 at 04:06:09AM +0200, Marko Kreen wrote:
> > > Well, actually they do. glibc in <stdint.h> and NetBSD in
> > > <sys/inttypes.h> which is a mess, all rigth. Problem is that
> > > postgres.h does not know about this. I guess that C99 forgot to
> > > specify _where_ they should be defined.
> >
> > Correction, they both have <inttypes.h> which probably is the right
> > location for this.
>
> <stdint.h> is adequate to pick up uint*_t. <inttypes.h> is defined to
> include <stdint.h>. Of course all this C99 stuff is new and existing
> implementations may have the typedefs in different files or not have
> them at all.
But as I said, NetBSD does not have it. So what is the
correct/portable/standard location for it? Can anyone with C99
standard in hand find that out?
E.g. Tom Lane has some OS where these types are in
std{io|lib|def|arg} but on NetBSD and glibc/Linux you must include
separate header file for them.
--
marko
Applied. Thanks. [ Charset ISO-8859-1 unsupported, converting... ] > > The KAME files md5.* and sha1.* have the following changelog > entry: > > ---------------------------- > revision 1.2 > date: 2000/12/04 01:20:38; author: tgl; state: Exp; lines: > +18 -18 > Eliminate some of the more blatant platform-dependencies ... it > builds here now, anyway ... > ---------------------------- > > Which basically changes u_int*_t -> uint*_t, so now it does not > compile neither under Debian 2.2 nor under NetBSD 1.5 which > is platform independent? all right. Also it replaces $KAME$ > with $Id$ which is Bad Thing. PostgreSQL Id should be added as a > separate line so the file history could be seen. > > So here is patch: > > * changes uint*_t -> uint*. I guess that was the original > intention > * adds uint64 type to include/c.h because its needed > [somebody should check if I did it right] > * adds back KAME Id, because KAME is the master repository > * removes stupid c++ comments in pgcrypto.c > * removes <sys/types.h> from the code, its not needed > > -- > marko > [ Attachment, skipping... ] -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000+ If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania19026