Обсуждение: patch: contrib/pgcrypto sanity

Поиск
Список
Период
Сортировка

patch: contrib/pgcrypto sanity

От
Marko Kreen
Дата:
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


Вложения

Re: patch: contrib/pgcrypto sanity

От
Tom Lane
Дата:
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


Re: patch: contrib/pgcrypto sanity

От
Marko Kreen
Дата:
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



Re: patch: contrib/pgcrypto sanity

От
Marko Kreen
Дата:
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



Re: patch: contrib/pgcrypto sanity

От
Pete Forman
Дата:
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.


Re: patch: contrib/pgcrypto sanity

От
Marko Kreen
Дата:
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



Re: patch: contrib/pgcrypto sanity

От
Bruce Momjian
Дата:
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