Thread: A couple items on TODO
As I was browsing TODO, I noticed a couple unassigned items that I may be able to help with (I haven't worked with the source before): *Add use of 'const' for variables in source tree *Convert remaining fprintf(stderr,...)/perror() to elog() Neither seemed to be active at all. I also noticed that this item has been there for a while: *Encrpyt passwords in pg_shadow table using MD5 (Bruce, Vince) Are the mentioned people actually working on the last item? All of the items seem to be simple and seperated enough that I might be able to learn the source as I go. I can see how the last one might be more difficult, but I don't know. As goes without saying, I really don't know much about postgres, so any kind of schedule is completely unknown. For a cursory glance, I used the postgresql-7.1beta6 source, but I will grab the latest to actually work on. Any input about the difficulties or affected areas of code would be appreciated. I suppose if the items made it to TODO, they will take more than a couple minutes to fix. Regards, Jeff Davis
> I also noticed that this item has been there for a while: > *Encrpyt passwords in pg_shadow table using MD5 (Bruce, Vince) While you are there do you think it's possible to make an mcrypt function? :)
> > I also noticed that this item has been there for a while: > > *Encrpyt passwords in pg_shadow table using MD5 (Bruce, Vince) > > While you are there do you think it's possible to make an mcrypt function? > :) See contrib/pgcrypto. -- 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
> As I was browsing TODO, I noticed a couple unassigned items that I may be > able to help with (I haven't worked with the source before): > > *Add use of 'const' for variables in source tree I would discuss this item with the hackers list and see exactly what people want done with it. > *Convert remaining fprintf(stderr,...)/perror() to elog() The issue here is that some calls can't use elog() because the context is not properly set up yet so we need to identify the non-elog error calls and figure out if they should be elog(). > > Neither seemed to be active at all. > > I also noticed that this item has been there for a while: > *Encrpyt passwords in pg_shadow table using MD5 (Bruce, Vince) This is done. I forgot to mark it. I just marked it now. -- 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
Jeff Davis writes: > *Convert remaining fprintf(stderr,...)/perror() to elog() This isn't quite as easy as a mechanical conversion, mind you, because elog of course has rather complex side effects besides printing out a message. What we'd need is some sort of option to print a message of a given category without taking these side effects. -- Peter Eisentraut peter_e@gmx.net http://funkturm.homeip.net/~peter
Peter Eisentraut <peter_e@gmx.net> writes:
> Jeff Davis writes:
>> *Convert remaining fprintf(stderr,...)/perror() to elog()
> This isn't quite as easy as a mechanical conversion, mind you, because
> elog of course has rather complex side effects besides printing out a
> message.
AFAIR, elog at NOTICE or DEBUG level isn't really supposed to have any
side-effects. The bigger issue is that you have to be careful about
using it in certain places, mainly during startup or for reporting
communication errors. (send failure -> elog -> tries to send message to
client -> send failure -> elog -> trouble)
Also, I believe most of the printf's in the backend are in debugging
support code that's not even compiled by default. The return on
investment from converting those routines to use elog is really nil.
There may be a few remaining printf calls that should be converted to
elog, but I don't think this is a big issue.
regards, tom lane
> AFAIR, elog at NOTICE or DEBUG level isn't really supposed to have any > side-effects. The bigger issue is that you have to be careful about > using it in certain places, mainly during startup or for reporting > communication errors. (send failure -> elog -> tries to send message to > client -> send failure -> elog -> trouble) > > Also, I believe most of the printf's in the backend are in debugging > support code that's not even compiled by default. The return on > investment from converting those routines to use elog is really nil. > There may be a few remaining printf calls that should be converted to > elog, but I don't think this is a big issue. Clearly not a big issue, but something someone can poke around at to get started. -- 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
Tom Lane writes: > AFAIR, elog at NOTICE or DEBUG level isn't really supposed to have any > side-effects. The bigger issue is that you have to be careful about > using it in certain places, mainly during startup or for reporting > communication errors. (send failure -> elog -> tries to send message to > client -> send failure -> elog -> trouble) It's especially postmaster.c and the related subroutines elsewhere that I'm not happy about. The postmaster would really need a way to report an error to the log and continue in its merry ways. This could probably be encapsulated in elog() by setting a flag variable "I'm the postmaster" -- might even exit already. Note: In my experience, the previous suggestion to return to the postmaster main loop on error would not really be useful. -- Peter Eisentraut peter_e@gmx.net http://funkturm.homeip.net/~peter
> > As I was browsing TODO, I noticed a couple unassigned items > that I may be > > able to help with (I haven't worked with the source before): > > > > *Add use of 'const' for variables in source tree > > I would discuss this item with the hackers list and see exactly what > people want done with it. I have noticed while working on command.c and heap.c that half the functions pass 'const char *' and the other half pass just 'char *'. This is a pain when you have a little helper function like 'is_relation(char *)' and you want to pass a 'const char *' to it and vice versa. ie. Compiler warnings - it's sort of annoying. Chris
> > > As I was browsing TODO, I noticed a couple unassigned items > > that I may be > > > able to help with (I haven't worked with the source before): > > > > > > *Add use of 'const' for variables in source tree > > > > I would discuss this item with the hackers list and see exactly what > > people want done with it. > > I have noticed while working on command.c and heap.c that half the functions > pass 'const char *' and the other half pass just 'char *'. This is a pain > when you have a little helper function like 'is_relation(char *)' and you > want to pass a 'const char *' to it and vice versa. ie. Compiler warnings - > it's sort of annoying. > Yes, it can be very annoying. -- 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
"Christopher Kings-Lynne" <chriskl@familyhealth.com.au> writes:
> I have noticed while working on command.c and heap.c that half the functions
> pass 'const char *' and the other half pass just 'char *'. This is a pain
Yeah, people have started to use 'const' in new code, but the older
stuff doesn't use it, which means that the net effect is probably
more annoyance than help. I'm afraid that if we attack this in an
incremental way, we'll end up with code that may have a lot of const
markers in the declarations, but the actual code is riddled with
explicit casts to remove const because at one time or another that
was necessary in a particular place.
Can anyone think of a way to get from here to there without either
a lot of leftover cruft, or a "big bang" massive changeover?
regards, tom lane
> > > *Add use of 'const' for variables in source tree > > > > I would discuss this item with the hackers list and see exactly what > > people want done with it. > > I have noticed while working on command.c and heap.c that half the > functions pass 'const char *' and the other half pass just 'char *'. This > is a pain when you have a little helper function like 'is_relation(char *)' > and you want to pass a 'const char *' to it and vice versa. ie. Compiler > warnings - it's sort of annoying. > That's good information, now I have a better idea what I am looking for. I am using Source Navigator (good recommendation I got reading this list). I am basically just trying to find either variables that can be declared const, or inconsistancies (as Chris mentions). If anyone else has a clearer idea of what the intention of that todo item is, let me know. Jeff
> That's good information, now I have a better idea what I am > looking for. I am > using Source Navigator (good recommendation I got reading this > list). I am > basically just trying to find either variables that can be > declared const, or > inconsistancies (as Chris mentions). > > If anyone else has a clearer idea of what the intention of that > todo item is, > let me know. I assume that since most of the times char * is passed to a function, it is supposed to be unmodifiable. Putting the 'const' there enforces this - thereby making PostgreSQL more robust against poxy programming. Chris
Tom Lane writes: > Yeah, people have started to use 'const' in new code, but the older > stuff doesn't use it, which means that the net effect is probably > more annoyance than help. I'm afraid that if we attack this in an > incremental way, we'll end up with code that may have a lot of const > markers in the declarations, but the actual code is riddled with > explicit casts to remove const because at one time or another that > was necessary in a particular place. > > Can anyone think of a way to get from here to there without either > a lot of leftover cruft, or a "big bang" massive changeover? What I usually do if I feel a parameter could be made const is to propagate the change as far as necessary to the underlying functions. From time to time this turns out to be impossible at some layer. BUT: This is an indicator that you really don't know whether the value is const so you shouldn't declare it thus. IMHO, a better project than putting const qualifiers all over interfaces that you are not familiar with would be to clean up all the -Wcast-qual warnings. -- Peter Eisentraut peter_e@gmx.net http://funkturm.homeip.net/~peter
> That's good information, now I have a better idea what I am looking for. I am > using Source Navigator (good recommendation I got reading this list). I am > basically just trying to find either variables that can be declared const, or > inconsistancies (as Chris mentions). > > If anyone else has a clearer idea of what the intention of that todo item is, > let me know. This is definately the intent of the TODO item. -- 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
Tom Lane <tgl@sss.pgh.pa.us> writes: > "Christopher Kings-Lynne" <chriskl@familyhealth.com.au> writes: > > I have noticed while working on command.c and heap.c that half the functions > > pass 'const char *' and the other half pass just 'char *'. This is a pain > > Yeah, people have started to use 'const' in new code, but the older > stuff doesn't use it, which means that the net effect is probably > more annoyance than help. I'm afraid that if we attack this in an > incremental way, we'll end up with code that may have a lot of const > markers in the declarations, but the actual code is riddled with > explicit casts to remove const because at one time or another that > was necessary in a particular place. > > Can anyone think of a way to get from here to there without either > a lot of leftover cruft, or a "big bang" massive changeover? You don't need a flag day, if that's what you mean. You just start adding const at the lowest levels and grow them upwards. I've done it before in large old projects. It's easy to do a few functions as a fairly mindless task. You get there eventually. The main trick is to never add casts merely to avoid warnings (they will never get removed and are a potential source of future bugs), and to make sure that new functions use const wherever possible (so you don't move backward over time). It's worth it in the long run because it gives another way to catch stupid bugs at compile time. Ian