Thread: clean up assertion code
This patch removes a lot of unused code related to assertions and error handling, and simplifies the code that remains. Apparently, the code that left Berkeley had a whole "error handling subsystem", which exceptions and whatnot. Since we don't use that anymore, there's no reason to keep it around. The regression tests pass with the patch applied. Unless anyone sees a problem, please apply. Cheers, Neil -- Neil Conway <neilconway@rogers.com> PGP Key ID: DB3C29FC
Attachment
nconway@klamath.dyndns.org (Neil Conway) writes:
> This patch removes a lot of unused code related to assertions and
> error handling, and simplifies the code that remains. Apparently,
> the code that left Berkeley had a whole "error handling subsystem",
> which exceptions and whatnot. Since we don't use that anymore,
> there's no reason to keep it around.
While I don't have a problem with this idea, the patch is poorly
presented: if applied as-is, it will reduce the no-longer-needed files
to zero lines, rather than deleting them as intended.
When you want a patch to add or remove files, please list the added
or removed files separately, rather than presenting it like this.
It's much too easy for the committer to miss the need for "cvs add"
and "cvs remove" commands otherwise.
regards, tom lane
On Mon, Jul 29, 2002 at 04:01:07PM -0400, Tom Lane wrote: > When you want a patch to add or remove files, please list the added > or removed files separately, rather than presenting it like this. > It's much too easy for the committer to miss the need for "cvs add" > and "cvs remove" commands otherwise. Ah, I had thought CVS would figure that out, but I guess not. The removed files are: src/backend/utils/error/exc.c src/backend/utils/error/excabort.c src/backend/utils/error/excid.c src/backend/utils/error/format.c src/include/utils/exc.h src/include/utils/excid.h Cheers, Neil -- Neil Conway <neilconway@rogers.com> PGP Key ID: DB3C29FC
Neil Conway wrote: > On Mon, Jul 29, 2002 at 04:01:07PM -0400, Tom Lane wrote: > > When you want a patch to add or remove files, please list the added > > or removed files separately, rather than presenting it like this. > > It's much too easy for the committer to miss the need for "cvs add" > > and "cvs remove" commands otherwise. > > Ah, I had thought CVS would figure that out, but I guess not. The > removed files are: > > src/backend/utils/error/exc.c > src/backend/utils/error/excabort.c > src/backend/utils/error/excid.c > src/backend/utils/error/format.c > src/include/utils/exc.h > src/include/utils/excid.h > I am not sure it does. I will let you know. -- 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, Pennsylvania 19026
nconway@klamath.dyndns.org (Neil Conway) writes: > The regression tests pass with the patch applied. Unless anyone > sees a problem, please apply. Can this patch be applied? If there are any outstanding issues with it, let me know -- I'm not aware of any. Cheers, Neil -- Neil Conway <neilconway@rogers.com> PGP Key ID: DB3C29FC
Neil Conway <nconway@klamath.dyndns.org> writes:
> nconway@klamath.dyndns.org (Neil Conway) writes:
>> The regression tests pass with the patch applied. Unless anyone
>> sees a problem, please apply.
> Can this patch be applied? If there are any outstanding issues with
> it, let me know -- I'm not aware of any.
It looked all right to me, just on an eyeball test. Did you verify that
an actual assertion trap still behaves as desired (ie, print to stderr
and dump core)?
regards, tom lane
Tom Lane <tgl@sss.pgh.pa.us> writes: > It looked all right to me, just on an eyeball test. Did you verify that > an actual assertion trap still behaves as desired (ie, print to stderr > and dump core)? Yes, that behavior should be unchanged. Cheers, Neil -- Neil Conway <neilconway@rogers.com> PGP Key ID: DB3C29FC
Your patch has been added to the PostgreSQL unapplied patches list at:
http://candle.pha.pa.us/cgi-bin/pgpatches
I will try to apply it within the next 48 hours.
---------------------------------------------------------------------------
Neil Conway wrote:
> This patch removes a lot of unused code related to assertions and
> error handling, and simplifies the code that remains. Apparently,
> the code that left Berkeley had a whole "error handling subsystem",
> which exceptions and whatnot. Since we don't use that anymore,
> there's no reason to keep it around.
>
> The regression tests pass with the patch applied. Unless anyone
> sees a problem, please apply.
>
> Cheers,
>
> Neil
>
> --
> Neil Conway <neilconway@rogers.com>
> PGP Key ID: DB3C29FC
[ Attachment, skipping... ]
>
> ---------------------------(end of broadcast)---------------------------
> TIP 5: Have you checked our extensive FAQ?
>
> http://www.postgresql.org/users-lounge/docs/faq.html
--
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, Pennsylvania 19026
Patch applied. Thanks. Several utils/error/* files were removed as part of this patch. --------------------------------------------------------------------------- Neil Conway wrote: > This patch removes a lot of unused code related to assertions and > error handling, and simplifies the code that remains. Apparently, > the code that left Berkeley had a whole "error handling subsystem", > which exceptions and whatnot. Since we don't use that anymore, > there's no reason to keep it around. > > The regression tests pass with the patch applied. Unless anyone > sees a problem, please apply. > > Cheers, > > Neil > > -- > Neil Conway <neilconway@rogers.com> > PGP Key ID: DB3C29FC [ Attachment, skipping... ] > > ---------------------------(end of broadcast)--------------------------- > TIP 5: Have you checked our extensive FAQ? > > http://www.postgresql.org/users-lounge/docs/faq.html -- 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
Actually, CVS did properly remove the files. The only problem was that the src/backend/utils/error/*.c files were specified backwards in the patch. The include/utils/*.h files removed automatically. --------------------------------------------------------------------------- Neil Conway wrote: > On Mon, Jul 29, 2002 at 04:01:07PM -0400, Tom Lane wrote: > > When you want a patch to add or remove files, please list the added > > or removed files separately, rather than presenting it like this. > > It's much too easy for the committer to miss the need for "cvs add" > > and "cvs remove" commands otherwise. > > Ah, I had thought CVS would figure that out, but I guess not. The > removed files are: > > src/backend/utils/error/exc.c > src/backend/utils/error/excabort.c > src/backend/utils/error/excid.c > src/backend/utils/error/format.c > src/include/utils/exc.h > src/include/utils/excid.h > > Cheers, > > Neil > -- > Neil Conway <neilconway@rogers.com> > PGP Key ID: DB3C29FC > > ---------------------------(end of broadcast)--------------------------- > TIP 5: Have you checked our extensive FAQ? > > http://www.postgresql.org/users-lounge/docs/faq.html > -- 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
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Actually, CVS did properly remove the files. The only problem was that
> the src/backend/utils/error/*.c files were specified backwards in the
> patch. The include/utils/*.h files removed automatically.
Oh? Your commit message doesn't show that they went away...
it looks to me like three out of the six files Neil wanted to zap
are still with us.
regards, tom lane
I am running my standard post-patch application test to make sure the
files in my directory tree exactly match CVS, and I now see that CVS
re-added these two files:
U pgsql/src/include/utils/exc.h
U pgsql/src/include/utils/excid.h
Files removed. Seems 'patch' properly removes the file but CVS of
course doesn't. They have to be manually removed.
---------------------------------------------------------------------------
Tom Lane wrote:
> Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > Actually, CVS did properly remove the files. The only problem was that
> > the src/backend/utils/error/*.c files were specified backwards in the
> > patch. The include/utils/*.h files removed automatically.
>
> Oh? Your commit message doesn't show that they went away...
> it looks to me like three out of the six files Neil wanted to zap
> are still with us.
>
> regards, tom lane
>
--
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