The following review has been posted through the commitfest application:
make installcheck-world: not tested
Implements feature: not tested
Spec compliant: not tested
Documentation: not tested
First of all, this seems like fixing a valid issue, albeit, the probability of
somebody messing is low, but it is still better to fix this problem.
I've not tested the patch in any detail, however, there are a couple of
comments I have before I proceed on with detailed testing.
1. pgindent is showing a few issues with formatting. Please have a look and
resolve those.
2. I think you can potentially use "len" variable instead of introducing
"buflen" and "tmplen" variables. Also, I would choose a more appropriate name
for "tmp" variable.
I believe if you move the following lines before the conditional statement and
simply and change the if statement to "if (len >= sizeof(buf) - 1)", it will
serve the purpose.
========================================
/* strip trailing newline and carriage return */
len = pg_strip_crlf(buf);
if (len == 0)
continue;
========================================
So, the patch should look like this in my opinion (ignore the formatting issues
as this is just to give you an idea of what I mean):
diff --git a/src/interfaces/libpq/fe-connect.c
b/src/interfaces/libpq/fe-connect.c
index 408000a..6ca262f 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -6949,6 +6949,7 @@ passwordFromFile(const char *hostname, const char *port,
const char *dbname,
{
FILE *fp;
struct stat stat_buf;
+ int line_number = 0;
#define LINELEN NAMEDATALEN*5
char buf[LINELEN];
@@ -7018,10 +7019,40 @@ passwordFromFile(const char *hostname, const char
*port, const char *dbname,
if (fgets(buf, sizeof(buf), fp) == NULL)
break;
- /* strip trailing newline and carriage return */
- len = pg_strip_crlf(buf);
+ line_number++;
- if (len == 0)
+ /* strip trailing newline and carriage return */
+ len = pg_strip_crlf(buf);
+
+ if (len == 0)
+ continue;
+
+ if (len >= sizeof(buf) - 1)
+ {
+ char tmp[LINELEN];
+
+ /*
+ * Warn if this password setting line is too long,
+ * because it's unexpectedly truncated.
+ */
+ if (buf[0] != '#')
+ fprintf(stderr,
+ libpq_gettext("WARNING: line %d
too long in password file \"%s\"\n"),
+ line_number, pgpassfile);
+
+ /* eat rest of the line */
+ while (!feof(fp) && !ferror(fp))
+ {
+ if (fgets(tmp, sizeof(tmp), fp) == NULL)
+ break;
+ len = strlen(tmp);
+ if (len < sizeof(tmp) -1 || tmp[len - 1] ==
'\n')
+ break;
+ }
+ }
+
+ /* ignore comments */
+ if (buf[0] == '#')
---
Highgo Software (Canada/China/Pakistan)
URL : www.highgo.ca
ADDR: 10318 WHALLEY BLVD, Surrey, BC
CELL：+923335449950 EMAIL: mailto:[email protected]
SKYPE: engineeredvirus
The new status of this patch is: Waiting on Author
