I am addressing these issues with my programmers now. Should see better results next week.
Sincerely,
Joshua D. Drake
Neil Conway wrote:
This code is pretty awful, IMHO.
If you're going to copy code from a 3rd party (in this case, MSDN), it is standard practise to include an attribution. Also, what redistribution terms apply to MSDN sample code?
Assuming we don't remove or rewrite the MSDN sample code, it is usually considered good practise to put imported code into a separate file.
You can remove the kludge for 16-bit processes, as PostgreSQL won't ever be one. Dunno if it's worth modifying the sample code for this.
It is generally considered good practise to divide distinct functionality into separate functions, rather than use a ~450 line main() function.
I'd like to see fewer WIN32 #ifdefs in the main code path; please also endeavour to make them as small as possible.
Copying and pasting code from one branch of an #ifdef or if into the other should also be avoided where possible: this is done in a couple places.
Other random badness I noticed while browsing through the code:
#ifdef WIN32 char *bindir="c:\\pgsql\\bin"; char *VERSION="7.3.5"; #else char *bindir="@bindir@"; char *VERSION="@VERSION@"; #endif
This is plainly wrong.
char* DEFPOSTOPTS= NULL; char* POSTOPTSFILE= NULL; char* PIDFILE= NULL;
int n_sig;
char *logfile=NULL; char *silence_echo=NULL; char *shutdown_mode="smart"; char *op=NULL; int PID;
Can we please try to follow *some* kind of coherent convention for naming and indentation?
char* buffer; char* buffer2;
Please put some thought into your variable names.
po_path=(char*)malloc(sizeof(char)* (strlen(PGPATH)+strlen(pgsqlexe)+2)); sprintf(po_path, "%s%s%s",PGPATH, path_delim, pgsqlexe);
This code needlessly assumes strlen(path_delim) == 1. On the other hand, it is reasonable to assume sizeof(char) == 1 (this is guaranteed by ANSI C).
for ( ;cont; token = (char*)strtok(NULL, env_delim)){ if (token==NULL) cont=0; else { buffer=(char*)malloc(sizeof(char)* (strlen(token)+strlen(CMDNAME)+1)); sprintf(buffer, token); strcat (buffer, path_delim); strcat (buffer, CMDNAME); if (post_file=fopen(buffer, "r")){ self_path=token; cont=0; free(buffer); } else free(buffer); } if (token==NULL) cont=0; }
Good lord.
-Neil
--
Command Prompt, Inc., home of Mammoth PostgreSQL - S/ODBC and S/JDBC
Postgresql support, programming shared hosting and dedicated hosting.
+1-503-222-2783 - [EMAIL PROTECTED] - http://www.commandprompt.com
Editor-N-Chief - PostgreSQl.Org - http://www.postgresql.org
---------------------------(end of broadcast)--------------------------- TIP 2: you can get off all lists at once with the unregister command (send "unregister YourEmailAddressHere" to [EMAIL PROTECTED])