[PATCH] introduce and use xmalloc_fgets/fgetline
by Denys Vlasenko
This fixes problems of having long lines truncated - and we do have very long lines
sometimes - curl errors with HTML, list of debuginfos etc.
--
vda
diff -x '*.po' -d -urpN abrt.6/lib/plugins/Bugzilla.cpp abrt.7/lib/plugins/Bugzilla.cpp
--- abrt.6/lib/plugins/Bugzilla.cpp 2010-10-21 15:55:12.000000000 +0200
+++ abrt.7/lib/plugins/Bugzilla.cpp 2010-10-21 21:04:11.242864263 +0200
@@ -120,21 +120,22 @@ string CReporterBugzilla::Report(const m
/* Consume log from stdout */
string bug_status;
- char buf[512];
- while (fgets(buf, sizeof(buf), fp))
+ char *buf;
+ while ((buf = xmalloc_fgetline(fp)) != NULL)
{
- strchrnul(buf, '\n')[0] = '\0';
if (strncmp(buf, "STATUS:", 7) == 0)
bug_status = buf + 7;
else
if (strncmp(buf, "EXCEPT:", 7) == 0)
{
+ CABRTException e(EXCEP_PLUGIN, "%s", buf + 7);
+ free(buf);
fclose(fp);
waitpid(pid, NULL, 0);
- throw CABRTException(EXCEP_PLUGIN, "%s", buf + 7);
+ throw e;
}
- else
- update_client("%s", buf);
+ update_client("%s", buf);
+ free(buf);
}
fclose(fp); /* this also closes pipefds[0] */
diff -x '*.po' -d -urpN abrt.6/lib/plugins/CCpp.cpp abrt.7/lib/plugins/CCpp.cpp
--- abrt.6/lib/plugins/CCpp.cpp 2010-10-21 15:55:12.000000000 +0200
+++ abrt.7/lib/plugins/CCpp.cpp 2010-10-21 21:02:03.057864128 +0200
@@ -154,21 +154,14 @@ static char *install_debug_infos(const c
return NULL;
}
- /* With 126 debuginfos I've seen lines 9k+ chars long...
- * yet, having it truly unlimited is bad too,
- * therefore we are using LARGE, but still limited buffer.
- */
- char *buff = (char*) xmalloc(64*1024);
-
+ char *buff;
struct strbuf *buf_build_ids = strbuf_new();
-
- while (fgets(buff, 64*1024, pipeout_fp))
+ while ((buff = xmalloc_fgetline(pipeout_fp)) != NULL)
{
- strchrnul(buff, '\n')[0] = '\0';
-
if (strncmp(buff, "MISSING:", 8) == 0)
{
strbuf_append_strf(buf_build_ids, "Debuginfo absent: %s\n", buff + 8);
+ free(buff);
continue;
}
@@ -183,8 +176,8 @@ static char *install_debug_infos(const c
VERB1 log("%s", buff);
update_client("%s", buff);
}
+ free(buff);
}
- free(buff);
fclose(pipeout_fp);
int status = 0;
diff -x '*.po' -d -urpN abrt.6/lib/plugins/FileTransfer.cpp abrt.7/lib/plugins/FileTransfer.cpp
--- abrt.6/lib/plugins/FileTransfer.cpp 2010-10-21 15:55:12.000000000 +0200
+++ abrt.7/lib/plugins/FileTransfer.cpp 2010-10-21 21:37:38.648149042 +0200
@@ -280,14 +280,13 @@ void CFileTransfer::Run(const char *pAct
goto del_tmp_dir;
}
- char dirname[PATH_MAX];
- while (fgets(dirname, sizeof(dirname), dirlist) != NULL)
+ char *dirname;
+ while ((dirname = xmalloc_fgetline(dirlist)) != NULL)
{
- strchrnul(dirname, '\n')[0] = '\0';
string archivename = ssprintf("%s/%s-%s%s", tmpdir_name, hostname, DirBase(dirname).c_str(), m_sArchiveType.c_str());
try
{
- VERB3 log("Creating archive '%s' of dir '%s'", archivename.c_str(), dirname);
+ VERB3 log("Creating archive '%s' of dir '%s'", archivename.c_str(), dirname);
CreateArchive(archivename.c_str(), dirname);
VERB3 log("Sending archive to '%s'", m_sURL.c_str());
SendFile(m_sURL.c_str(), archivename.c_str());
@@ -298,6 +297,7 @@ void CFileTransfer::Run(const char *pAct
}
VERB3 log("Deleting archive '%s'", archivename.c_str());
unlink(archivename.c_str());
+ free(dirname);
}
fclose(dirlist);
diff -x '*.po' -d -urpN abrt.6/lib/plugins/Logger.cpp abrt.7/lib/plugins/Logger.cpp
--- abrt.6/lib/plugins/Logger.cpp 2010-10-21 15:55:12.000000000 +0200
+++ abrt.7/lib/plugins/Logger.cpp 2010-10-21 21:43:27.216150678 +0200
@@ -105,10 +105,10 @@ string CLogger::Report(const map_crash_d
FILE *fp = fdopen(pipefds[0], "r");
if (!fp)
die_out_of_memory();
- char buf[512];
- while (fgets(buf, sizeof(buf), fp))
+ char *buf;
+ while ((buf = xmalloc_fgets(fp)) != NULL)
{
- full_write_str(fd, buf);
+ full_write_str(fd, buf);
}
fclose(fp); /* this also closes pipefds[0] */
/* wait for child to actually exit, and prevent leaving a zombie behind */
diff -x '*.po' -d -urpN abrt.6/lib/plugins/ReportUploader.cpp abrt.7/lib/plugins/ReportUploader.cpp
--- abrt.6/lib/plugins/ReportUploader.cpp 2010-10-21 15:55:12.000000000 +0200
+++ abrt.7/lib/plugins/ReportUploader.cpp 2010-10-21 21:51:07.817149237 +0200
@@ -55,11 +55,11 @@ static string ReadCommand(const char *cm
}
string result;
- char buff[1024];
- while (fgets(buff, sizeof(buff), fp) != NULL)
+ char *buff;
+ while ((buff = xmalloc_fgetline(fp)) != NULL)
{
- strchrnul(buff, '\n')[0] = '\0';
result += buff;
+ free(buff);
}
int retcode = pclose(fp);
diff -x '*.po' -d -urpN abrt.6/lib/plugins/RHTSupport.cpp abrt.7/lib/plugins/RHTSupport.cpp
--- abrt.6/lib/plugins/RHTSupport.cpp 2010-10-21 15:55:12.000000000 +0200
+++ abrt.7/lib/plugins/RHTSupport.cpp 2010-10-21 21:41:07.671158618 +0200
@@ -111,22 +111,23 @@ string CReporterRHticket::Report(const m
die_out_of_memory();
/* Consume log from stdout */
- std::string bug_status;
- char buf[512];
- while (fgets(buf, sizeof(buf), fp))
+ string bug_status;
+ char *buf;
+ while ((buf = xmalloc_fgetline(fp)) != NULL)
{
- strchrnul(buf, '\n')[0] = '\0';
if (strncmp(buf, "STATUS:", 7) == 0)
bug_status = buf + 7;
else
if (strncmp(buf, "EXCEPT:", 7) == 0)
{
+ CABRTException e(EXCEP_PLUGIN, "%s", buf + 7);
+ free(buf);
fclose(fp);
waitpid(pid, NULL, 0);
- throw CABRTException(EXCEP_PLUGIN, "%s", buf + 7);
+ throw e;
}
- else
- update_client("%s", buf);
+ update_client("%s", buf);
+ free(buf);
}
fclose(fp); /* this also closes pipefds[0] */
diff -x '*.po' -d -urpN abrt.6/lib/utils/hooklib.c abrt.7/lib/utils/hooklib.c
--- abrt.6/lib/utils/hooklib.c 2010-10-21 15:55:12.000000000 +0200
+++ abrt.7/lib/utils/hooklib.c 2010-10-21 21:17:02.112864566 +0200
@@ -25,10 +25,10 @@ void parse_conf(const char *additional_c
if (!fp)
return;
- char line[256];
while (1)
{
- if (fgets(line, sizeof(line), fp) == NULL)
+ char *line = xmalloc_fgetline(fp);
+ if (!line)
{
fclose(fp);
if (additional_conf)
@@ -44,7 +44,6 @@ void parse_conf(const char *additional_c
break;
}
- strchrnul(line, '\n')[0] = '\0';
const char *p = skip_whitespace(line);
#undef DIRECTIVE
#define DIRECTIVE "MaxCrashReportsSize"
@@ -52,7 +51,7 @@ void parse_conf(const char *additional_c
{
p = skip_whitespace(p + sizeof(DIRECTIVE)-1);
if (*p != '=')
- continue;
+ goto free_line;
p = skip_whitespace(p + 1);
if (isdigit(*p))
{
@@ -60,7 +59,7 @@ void parse_conf(const char *additional_c
* kicks in first, and we don't "fight" with it. */
*setting_MaxCrashReportsSize = (unsigned long)xatou(p) * 5 / 4;
}
- continue;
+ goto free_line;
}
#undef DIRECTIVE
#define DIRECTIVE "MakeCompatCore"
@@ -68,10 +67,10 @@ void parse_conf(const char *additional_c
{
p = skip_whitespace(p + sizeof(DIRECTIVE)-1);
if (*p != '=')
- continue;
+ goto free_line;
p = skip_whitespace(p + 1);
*setting_MakeCompatCore = string_to_bool(p);
- continue;
+ goto free_line;
}
#undef DIRECTIVE
#define DIRECTIVE "SaveBinaryImage"
@@ -79,13 +78,16 @@ void parse_conf(const char *additional_c
{
p = skip_whitespace(p + sizeof(DIRECTIVE)-1);
if (*p != '=')
- continue;
+ goto free_line;
p = skip_whitespace(p + 1);
*setting_SaveBinaryImage = string_to_bool(p);
- continue;
+ goto free_line;
}
#undef DIRECTIVE
/* add more 'if (strncmp(p, DIRECTIVE, sizeof(DIRECTIVE)-1) == 0)' here... */
+
+ free_line:
+ free(line);
}
}
diff -x '*.po' -d -urpN abrt.6/lib/utils/Plugin.cpp abrt.7/lib/utils/Plugin.cpp
--- abrt.6/lib/utils/Plugin.cpp 2010-10-21 15:55:12.000000000 +0200
+++ abrt.7/lib/utils/Plugin.cpp 2010-10-21 22:03:42.409084142 +0200
@@ -56,7 +56,7 @@ const map_plugin_settings_t& CPlugin::Ge
}
bool LoadPluginSettings(const char *pPath, map_plugin_settings_t& pSettings,
- bool skipKeysWithoutValue /*= true*/)
+ bool skipKeysWithoutValue /*= true*/)
{
FILE *fp = stdin;
if (strcmp(pPath, "-") != 0)
@@ -66,16 +66,15 @@ bool LoadPluginSettings(const char *pPat
return false;
}
- char line[512];
- while (fgets(line, sizeof(line), fp))
+ char *line;
+ while ((line = xmalloc_fgetline(fp)) != NULL)
{
- strchrnul(line, '\n')[0] = '\0';
unsigned ii;
bool is_value = false;
bool valid = false;
bool in_quote = false;
- std::string key;
- std::string value;
+ std::string key;
+ std::string value;
for (ii = 0; line[ii] != '\0'; ii++)
{
if (line[ii] == '"')
@@ -106,23 +105,26 @@ bool LoadPluginSettings(const char *pPat
}
}
- /* Skip broken or empty lines. */
- if (!valid)
- continue;
+ /* Skip broken or empty lines. */
+ if (!valid)
+ goto free_line;
- /* Skip lines with empty key. */
- if (key.length() == 0)
- continue;
+ /* Skip lines with empty key. */
+ if (key.length() == 0)
+ goto free_line;
- if (skipKeysWithoutValue && value.length() == 0)
- continue;
+ if (skipKeysWithoutValue && value.length() == 0)
+ goto free_line;
- /* Skip lines with unclosed quotes. */
+ /* Skip lines with unclosed quotes. */
if (in_quote)
- continue;
+ goto free_line;
- pSettings[key] = value;
+ pSettings[key] = value;
+ free_line:
+ free(line);
}
+
if (fp != stdin)
fclose(fp);
return true;
diff -x '*.po' -d -urpN abrt.6/src/cli/report.cpp abrt.7/src/cli/report.cpp
--- abrt.6/src/cli/report.cpp 2010-10-21 15:55:12.000000000 +0200
+++ abrt.7/src/cli/report.cpp 2010-10-21 21:23:04.341614221 +0200
@@ -542,10 +542,9 @@ static bool ask_yesno(const char *questi
/* The response might take more than 1 char in non-latin scripts. */
const char *yes = _("y");
const char *no = _("N");
- char *full_question = xasprintf("%s [%s/%s]: ", question, yes, no);
- printf(full_question);
- free(full_question);
+ printf("%s [%s/%s]: ", question, yes, no);
fflush(NULL);
+
char answer[16];
fgets(answer, sizeof(answer), stdin);
/* Use strncmp here because the answer might contain a newline as
diff -x '*.po' -d -urpN abrt.6/src/daemon/Settings.cpp abrt.7/src/daemon/Settings.cpp
--- abrt.6/src/daemon/Settings.cpp 2010-10-21 15:55:12.000000000 +0200
+++ abrt.7/src/daemon/Settings.cpp 2010-10-21 21:59:18.340333683 +0200
@@ -347,14 +347,13 @@ static void LoadGPGKeys()
/* every line is one key
* FIXME: make it more robust, it doesn't handle comments
*/
- char line[512];
- while (fgets(line, sizeof(line), fp))
+ char *line;
+ while ((line = xmalloc_fgetline(fp)) != NULL)
{
if (line[0] == '/') // probably the begining of path, so let's handle it as a key
- {
- strchrnul(line, '\n')[0] = '\0';
- g_settings_setOpenGPGPublicKeys = g_list_append(g_settings_setOpenGPGPublicKeys, xstrdup(line));
- }
+ g_settings_setOpenGPGPublicKeys = g_list_append(g_settings_setOpenGPGPublicKeys, line);
+ else
+ free(line);
}
fclose(fp);
}
@@ -366,12 +365,11 @@ static void LoadGPGKeys()
*/
static int ReadConfigurationFromFile(FILE *fp)
{
- char line[512];
+ char *line;
std::string section;
int lineno = 0;
- while (fgets(line, sizeof(line), fp))
+ while ((line = xmalloc_fgetline(fp)) != NULL)
{
- strchrnul(line, '\n')[0] = '\0';
++lineno;
bool is_key = true;
bool is_section = false;
@@ -422,12 +420,12 @@ static int ReadConfigurationFromFile(FIL
continue;
}
value += line[ii];
- }
+ } /* for */
if (is_quote)
{
error_msg("abrt.conf: Invalid syntax on line %d", lineno);
- return 1; /* error */
+ goto return_error;
}
if (is_section)
@@ -435,9 +433,9 @@ static int ReadConfigurationFromFile(FIL
if (line[ii] != ']') /* section not closed */
{
error_msg("abrt.conf: Section not closed on line %d", lineno);
- return 1; /* error */
+ goto return_error;
}
- continue;
+ goto free_line;
}
if (is_key)
@@ -445,14 +443,14 @@ static int ReadConfigurationFromFile(FIL
if (!value.empty()) /* the key is stored in value */
{
error_msg("abrt.conf: Invalid syntax on line %d", lineno);
- return 1; /* error */
+ goto return_error;
}
- continue;
+ goto free_line;
}
- else if (key.empty()) /* A line without key: " = something" */
+ if (key.empty()) /* A line without key: " = something" */
{
error_msg("abrt.conf: Invalid syntax on line %d", lineno);
- return 1; /* error */
+ goto return_error;
}
if (section == SECTION_COMMON)
@@ -476,9 +474,14 @@ static int ReadConfigurationFromFile(FIL
else
{
error_msg("abrt.conf: Ignoring entry in invalid section [%s]", section.c_str());
+ return_error:
+ free(line);
return 1; /* error */
}
- }
+ free_line:
+ free(line);
+ } /* while */
+
return 0; /* success */
}
13 years, 7 months
[PATCH] introduce and use xmalloc_fgets and xmalloc_fgetline
by Denys Vlasenko
Jiri noticed that some messages get truncated.
The reason is that we use fgets to read lines
to fixed buffers, and those buffers are too small
(or rather, some lines are quite long).
This is the general solution to that problem:
read into malloced, growing buffer.
Run tested. Please review.
--
vda
diff -urpN abrt.5/inc/abrtlib.h abrt.7/inc/abrtlib.h
--- abrt.5/inc/abrtlib.h 2010-10-21 15:55:12.000000000 +0200
+++ abrt.7/inc/abrtlib.h 2010-10-21 19:29:42.551109459 +0200
@@ -92,6 +92,11 @@ char *append_to_malloced_string(char *ms
char* skip_whitespace(const char *s);
char* skip_non_whitespace(const char *s);
+/* A-la fgets, but malloced and of unlimited size */
+char *xmalloc_fgets(FILE *file);
+/* Similar, but removes trailing \n */
+char *xmalloc_fgetline(FILE *file);
+
/* On error, copyfd_XX prints error messages and returns -1 */
enum {
COPYFD_SPARSE = 1 << 0,
diff -urpN abrt.5/lib/plugins/Bugzilla.cpp abrt.7/lib/plugins/Bugzilla.cpp
--- abrt.5/lib/plugins/Bugzilla.cpp 2010-10-21 15:55:12.000000000 +0200
+++ abrt.7/lib/plugins/Bugzilla.cpp 2010-10-21 21:04:11.242864263 +0200
@@ -120,21 +120,22 @@ string CReporterBugzilla::Report(const m
/* Consume log from stdout */
string bug_status;
- char buf[512];
- while (fgets(buf, sizeof(buf), fp))
+ char *buf;
+ while ((buf = xmalloc_fgetline(fp)) != NULL)
{
- strchrnul(buf, '\n')[0] = '\0';
if (strncmp(buf, "STATUS:", 7) == 0)
bug_status = buf + 7;
else
if (strncmp(buf, "EXCEPT:", 7) == 0)
{
+ CABRTException e(EXCEP_PLUGIN, "%s", buf + 7);
+ free(buf);
fclose(fp);
waitpid(pid, NULL, 0);
- throw CABRTException(EXCEP_PLUGIN, "%s", buf + 7);
+ throw e;
}
- else
- update_client("%s", buf);
+ update_client("%s", buf);
+ free(buf);
}
fclose(fp); /* this also closes pipefds[0] */
diff -urpN abrt.5/lib/plugins/CCpp.cpp abrt.7/lib/plugins/CCpp.cpp
--- abrt.5/lib/plugins/CCpp.cpp 2010-10-21 15:55:12.000000000 +0200
+++ abrt.7/lib/plugins/CCpp.cpp 2010-10-21 21:02:03.057864128 +0200
@@ -154,21 +154,14 @@ static char *install_debug_infos(const c
return NULL;
}
- /* With 126 debuginfos I've seen lines 9k+ chars long...
- * yet, having it truly unlimited is bad too,
- * therefore we are using LARGE, but still limited buffer.
- */
- char *buff = (char*) xmalloc(64*1024);
-
+ char *buff;
struct strbuf *buf_build_ids = strbuf_new();
-
- while (fgets(buff, 64*1024, pipeout_fp))
+ while ((buff = xmalloc_fgetline(pipeout_fp)) != NULL)
{
- strchrnul(buff, '\n')[0] = '\0';
-
if (strncmp(buff, "MISSING:", 8) == 0)
{
strbuf_append_strf(buf_build_ids, "Debuginfo absent: %s\n", buff + 8);
+ free(buff);
continue;
}
@@ -183,8 +176,8 @@ static char *install_debug_infos(const c
VERB1 log("%s", buff);
update_client("%s", buff);
}
+ free(buff);
}
- free(buff);
fclose(pipeout_fp);
int status = 0;
diff -urpN abrt.5/lib/plugins/FileTransfer.cpp abrt.7/lib/plugins/FileTransfer.cpp
--- abrt.5/lib/plugins/FileTransfer.cpp 2010-10-21 15:55:12.000000000 +0200
+++ abrt.7/lib/plugins/FileTransfer.cpp 2010-10-21 21:37:38.648149042 +0200
@@ -280,14 +280,13 @@ void CFileTransfer::Run(const char *pAct
goto del_tmp_dir;
}
- char dirname[PATH_MAX];
- while (fgets(dirname, sizeof(dirname), dirlist) != NULL)
+ char *dirname;
+ while ((dirname = xmalloc_fgetline(dirlist)) != NULL)
{
- strchrnul(dirname, '\n')[0] = '\0';
string archivename = ssprintf("%s/%s-%s%s", tmpdir_name, hostname, DirBase(dirname).c_str(), m_sArchiveType.c_str());
try
{
- VERB3 log("Creating archive '%s' of dir '%s'", archivename.c_str(), dirname);
+ VERB3 log("Creating archive '%s' of dir '%s'", archivename.c_str(), dirname);
CreateArchive(archivename.c_str(), dirname);
VERB3 log("Sending archive to '%s'", m_sURL.c_str());
SendFile(m_sURL.c_str(), archivename.c_str());
@@ -298,6 +297,7 @@ void CFileTransfer::Run(const char *pAct
}
VERB3 log("Deleting archive '%s'", archivename.c_str());
unlink(archivename.c_str());
+ free(dirname);
}
fclose(dirlist);
diff -urpN abrt.5/lib/plugins/Logger.cpp abrt.7/lib/plugins/Logger.cpp
--- abrt.5/lib/plugins/Logger.cpp 2010-10-21 15:55:12.000000000 +0200
+++ abrt.7/lib/plugins/Logger.cpp 2010-10-21 21:43:27.216150678 +0200
@@ -105,10 +105,10 @@ string CLogger::Report(const map_crash_d
FILE *fp = fdopen(pipefds[0], "r");
if (!fp)
die_out_of_memory();
- char buf[512];
- while (fgets(buf, sizeof(buf), fp))
+ char *buf;
+ while ((buf = xmalloc_fgets(fp)) != NULL)
{
- full_write_str(fd, buf);
+ full_write_str(fd, buf);
}
fclose(fp); /* this also closes pipefds[0] */
/* wait for child to actually exit, and prevent leaving a zombie behind */
diff -urpN abrt.5/lib/plugins/ReportUploader.cpp abrt.7/lib/plugins/ReportUploader.cpp
--- abrt.5/lib/plugins/ReportUploader.cpp 2010-10-21 15:55:12.000000000 +0200
+++ abrt.7/lib/plugins/ReportUploader.cpp 2010-10-21 21:51:07.817149237 +0200
@@ -55,11 +55,11 @@ static string ReadCommand(const char *cm
}
string result;
- char buff[1024];
- while (fgets(buff, sizeof(buff), fp) != NULL)
+ char *buff;
+ while ((buff = xmalloc_fgetline(fp)) != NULL)
{
- strchrnul(buff, '\n')[0] = '\0';
result += buff;
+ free(buff);
}
int retcode = pclose(fp);
diff -urpN abrt.5/lib/plugins/RHTSupport.cpp abrt.7/lib/plugins/RHTSupport.cpp
--- abrt.5/lib/plugins/RHTSupport.cpp 2010-10-21 15:55:12.000000000 +0200
+++ abrt.7/lib/plugins/RHTSupport.cpp 2010-10-21 21:41:07.671158618 +0200
@@ -111,22 +111,23 @@ string CReporterRHticket::Report(const m
die_out_of_memory();
/* Consume log from stdout */
- std::string bug_status;
- char buf[512];
- while (fgets(buf, sizeof(buf), fp))
+ string bug_status;
+ char *buf;
+ while ((buf = xmalloc_fgetline(fp)) != NULL)
{
- strchrnul(buf, '\n')[0] = '\0';
if (strncmp(buf, "STATUS:", 7) == 0)
bug_status = buf + 7;
else
if (strncmp(buf, "EXCEPT:", 7) == 0)
{
+ CABRTException e(EXCEP_PLUGIN, "%s", buf + 7);
+ free(buf);
fclose(fp);
waitpid(pid, NULL, 0);
- throw CABRTException(EXCEP_PLUGIN, "%s", buf + 7);
+ throw e;
}
- else
- update_client("%s", buf);
+ update_client("%s", buf);
+ free(buf);
}
fclose(fp); /* this also closes pipefds[0] */
diff -urpN abrt.5/lib/utils/hooklib.c abrt.7/lib/utils/hooklib.c
--- abrt.5/lib/utils/hooklib.c 2010-10-21 15:55:12.000000000 +0200
+++ abrt.7/lib/utils/hooklib.c 2010-10-21 21:17:02.112864566 +0200
@@ -25,10 +25,10 @@ void parse_conf(const char *additional_c
if (!fp)
return;
- char line[256];
while (1)
{
- if (fgets(line, sizeof(line), fp) == NULL)
+ char *line = xmalloc_fgetline(fp);
+ if (!line)
{
fclose(fp);
if (additional_conf)
@@ -44,7 +44,6 @@ void parse_conf(const char *additional_c
break;
}
- strchrnul(line, '\n')[0] = '\0';
const char *p = skip_whitespace(line);
#undef DIRECTIVE
#define DIRECTIVE "MaxCrashReportsSize"
@@ -52,7 +51,7 @@ void parse_conf(const char *additional_c
{
p = skip_whitespace(p + sizeof(DIRECTIVE)-1);
if (*p != '=')
- continue;
+ goto free_line;
p = skip_whitespace(p + 1);
if (isdigit(*p))
{
@@ -60,7 +59,7 @@ void parse_conf(const char *additional_c
* kicks in first, and we don't "fight" with it. */
*setting_MaxCrashReportsSize = (unsigned long)xatou(p) * 5 / 4;
}
- continue;
+ goto free_line;
}
#undef DIRECTIVE
#define DIRECTIVE "MakeCompatCore"
@@ -68,10 +67,10 @@ void parse_conf(const char *additional_c
{
p = skip_whitespace(p + sizeof(DIRECTIVE)-1);
if (*p != '=')
- continue;
+ goto free_line;
p = skip_whitespace(p + 1);
*setting_MakeCompatCore = string_to_bool(p);
- continue;
+ goto free_line;
}
#undef DIRECTIVE
#define DIRECTIVE "SaveBinaryImage"
@@ -79,13 +78,16 @@ void parse_conf(const char *additional_c
{
p = skip_whitespace(p + sizeof(DIRECTIVE)-1);
if (*p != '=')
- continue;
+ goto free_line;
p = skip_whitespace(p + 1);
*setting_SaveBinaryImage = string_to_bool(p);
- continue;
+ goto free_line;
}
#undef DIRECTIVE
/* add more 'if (strncmp(p, DIRECTIVE, sizeof(DIRECTIVE)-1) == 0)' here... */
+
+ free_line:
+ free(line);
}
}
diff -urpN abrt.5/lib/utils/Makefile.am abrt.7/lib/utils/Makefile.am
--- abrt.5/lib/utils/Makefile.am 2010-10-21 15:55:12.000000000 +0200
+++ abrt.7/lib/utils/Makefile.am 2010-10-21 19:30:44.774859851 +0200
@@ -13,6 +13,7 @@ libABRTUtils_la_SOURCES = \
concat_path_file.c \
append_to_malloced_string.c \
encbase64.c \
+ stdio_helpers.c \
hash_md5.c hash_md5.h \
hash_sha1.c hash_sha1.h \
read_write.c read_write.h \
diff -urpN abrt.5/lib/utils/Plugin.cpp abrt.7/lib/utils/Plugin.cpp
--- abrt.5/lib/utils/Plugin.cpp 2010-10-21 15:55:12.000000000 +0200
+++ abrt.7/lib/utils/Plugin.cpp 2010-10-21 22:03:42.409084142 +0200
@@ -56,7 +56,7 @@ const map_plugin_settings_t& CPlugin::Ge
}
bool LoadPluginSettings(const char *pPath, map_plugin_settings_t& pSettings,
- bool skipKeysWithoutValue /*= true*/)
+ bool skipKeysWithoutValue /*= true*/)
{
FILE *fp = stdin;
if (strcmp(pPath, "-") != 0)
@@ -66,16 +66,15 @@ bool LoadPluginSettings(const char *pPat
return false;
}
- char line[512];
- while (fgets(line, sizeof(line), fp))
+ char *line;
+ while ((line = xmalloc_fgetline(fp)) != NULL)
{
- strchrnul(line, '\n')[0] = '\0';
unsigned ii;
bool is_value = false;
bool valid = false;
bool in_quote = false;
- std::string key;
- std::string value;
+ std::string key;
+ std::string value;
for (ii = 0; line[ii] != '\0'; ii++)
{
if (line[ii] == '"')
@@ -106,23 +105,26 @@ bool LoadPluginSettings(const char *pPat
}
}
- /* Skip broken or empty lines. */
- if (!valid)
- continue;
-
- /* Skip lines with empty key. */
- if (key.length() == 0)
- continue;
+ /* Skip broken or empty lines. */
+ if (!valid)
+ goto free_line;
+
+ /* Skip lines with empty key. */
+ if (key.length() == 0)
+ goto free_line;
- if (skipKeysWithoutValue && value.length() == 0)
- continue;
+ if (skipKeysWithoutValue && value.length() == 0)
+ goto free_line;
- /* Skip lines with unclosed quotes. */
+ /* Skip lines with unclosed quotes. */
if (in_quote)
- continue;
+ goto free_line;
- pSettings[key] = value;
+ pSettings[key] = value;
+ free_line:
+ free(line);
}
+
if (fp != stdin)
fclose(fp);
return true;
diff -urpN abrt.5/lib/utils/stdio_helpers.c abrt.7/lib/utils/stdio_helpers.c
--- abrt.5/lib/utils/stdio_helpers.c 1970-01-01 01:00:00.000000000 +0100
+++ abrt.7/lib/utils/stdio_helpers.c 2010-10-21 19:30:27.927859415 +0200
@@ -0,0 +1,69 @@
+/*
+ * Utility routines.
+ *
+ * Copyright (C) 2001 Matt Krai
+ * Copyright (C) 2004 Erik Andersen <andersen(a)codepoet.org>
+ * Copyright (C) 2005, 2006 Rob Landley <rob(a)landley.net>
+ * Copyright (C) 2010 ABRT Team
+ *
+ * Licensed under GPLv2 or later, see file LICENSE in this source tree.
+ */
+#include "abrtlib.h"
+
+//TODO: add sanitizing upper limit (e.g 64K, 1M, or configurable).
+//This is why we don't use GNU's getline: it doesn't have
+//any upper sanity bound on line size.
+
+static char *xmalloc_fgets_internal(FILE *file, int *sizep)
+{
+ unsigned idx = 0;
+ char *linebuf = NULL;
+
+ while (1) {
+ char *r;
+
+ linebuf = xrealloc(linebuf, idx + 0x100);
+ r = fgets(&linebuf[idx], 0x100, file);
+ if (!r) {
+ /* need to terminate the line */
+ linebuf[idx] = '\0';
+ break;
+ }
+
+ /* stupid. fgets knows the len, it should report it somehow */
+ unsigned len = strlen(&linebuf[idx]);
+
+ idx += len;
+ if (len < 0xff || linebuf[idx - 1] == '\n')
+ break; /* we found \n or EOF */
+ }
+
+ *sizep = idx;
+
+ if (!idx) {
+ /* The very first fgets returned NULL. It's EOF (or error) */
+ free(linebuf);
+ linebuf = NULL;
+ }
+ return linebuf;
+}
+
+char *xmalloc_fgets(FILE *file)
+{
+ int sz;
+ char *r = xmalloc_fgets_internal(file, &sz);
+ if (!r)
+ return r;
+ return xrealloc(r, sz + 1);
+}
+
+char *xmalloc_fgetline(FILE *file)
+{
+ int sz;
+ char *r = xmalloc_fgets_internal(file, &sz);
+ if (!r)
+ return r;
+ if (r[sz - 1] == '\n')
+ r[--sz] = '\0';
+ return xrealloc(r, sz + 1);
+}
diff -urpN abrt.5/src/cli/report.cpp abrt.7/src/cli/report.cpp
--- abrt.5/src/cli/report.cpp 2010-10-21 15:55:12.000000000 +0200
+++ abrt.7/src/cli/report.cpp 2010-10-21 21:23:04.341614221 +0200
@@ -542,10 +542,9 @@ static bool ask_yesno(const char *questi
/* The response might take more than 1 char in non-latin scripts. */
const char *yes = _("y");
const char *no = _("N");
- char *full_question = xasprintf("%s [%s/%s]: ", question, yes, no);
- printf(full_question);
- free(full_question);
+ printf("%s [%s/%s]: ", question, yes, no);
fflush(NULL);
+
char answer[16];
fgets(answer, sizeof(answer), stdin);
/* Use strncmp here because the answer might contain a newline as
diff -urpN abrt.5/src/daemon/Settings.cpp abrt.7/src/daemon/Settings.cpp
--- abrt.5/src/daemon/Settings.cpp 2010-10-21 15:55:12.000000000 +0200
+++ abrt.7/src/daemon/Settings.cpp 2010-10-21 21:59:18.340333683 +0200
@@ -347,14 +347,13 @@ static void LoadGPGKeys()
/* every line is one key
* FIXME: make it more robust, it doesn't handle comments
*/
- char line[512];
- while (fgets(line, sizeof(line), fp))
+ char *line;
+ while ((line = xmalloc_fgetline(fp)) != NULL)
{
if (line[0] == '/') // probably the begining of path, so let's handle it as a key
- {
- strchrnul(line, '\n')[0] = '\0';
- g_settings_setOpenGPGPublicKeys = g_list_append(g_settings_setOpenGPGPublicKeys, xstrdup(line));
- }
+ g_settings_setOpenGPGPublicKeys = g_list_append(g_settings_setOpenGPGPublicKeys, line);
+ else
+ free(line);
}
fclose(fp);
}
@@ -366,12 +365,11 @@ static void LoadGPGKeys()
*/
static int ReadConfigurationFromFile(FILE *fp)
{
- char line[512];
+ char *line;
std::string section;
int lineno = 0;
- while (fgets(line, sizeof(line), fp))
+ while ((line = xmalloc_fgetline(fp)) != NULL)
{
- strchrnul(line, '\n')[0] = '\0';
++lineno;
bool is_key = true;
bool is_section = false;
@@ -422,12 +420,12 @@ static int ReadConfigurationFromFile(FIL
continue;
}
value += line[ii];
- }
+ } /* for */
if (is_quote)
{
error_msg("abrt.conf: Invalid syntax on line %d", lineno);
- return 1; /* error */
+ goto return_error;
}
if (is_section)
@@ -435,9 +433,9 @@ static int ReadConfigurationFromFile(FIL
if (line[ii] != ']') /* section not closed */
{
error_msg("abrt.conf: Section not closed on line %d", lineno);
- return 1; /* error */
+ goto return_error;
}
- continue;
+ goto free_line;
}
if (is_key)
@@ -445,14 +443,14 @@ static int ReadConfigurationFromFile(FIL
if (!value.empty()) /* the key is stored in value */
{
error_msg("abrt.conf: Invalid syntax on line %d", lineno);
- return 1; /* error */
+ goto return_error;
}
- continue;
+ goto free_line;
}
- else if (key.empty()) /* A line without key: " = something" */
+ if (key.empty()) /* A line without key: " = something" */
{
error_msg("abrt.conf: Invalid syntax on line %d", lineno);
- return 1; /* error */
+ goto return_error;
}
if (section == SECTION_COMMON)
@@ -476,9 +474,14 @@ static int ReadConfigurationFromFile(FIL
else
{
error_msg("abrt.conf: Ignoring entry in invalid section [%s]", section.c_str());
+ return_error:
+ free(line);
return 1; /* error */
}
- }
+ free_line:
+ free(line);
+ } /* while */
+
return 0; /* success */
}
13 years, 7 months
