Jim Meyering wrote:
> Jim Meyering wrote:
>> Under memory pressure, the use of strdup in strstr_rs could return NULL,
>> and that return value would then be dereferenced.
>>
>> The following change replaces that function with this far
>> simpler one and removes a second copy of the original.
>>
>> char *strstr_rs (const char *haystack, int byte)
>> {
>> const char *end_address = strchr (haystack, byte);
>> if (end_address) {
>> end_address += 1; /* skip past { or = */
>> end_address += strspn (end_address, " \t");
>> }
>>
>> return ((char *) end_address);
>> }
>>
>> Since the goal was just to remove the potential NULL-dereference,
>> I tried to keep this small, I didn't rename the function (it now looks
>> more like strchr than strstr), but can do that, too, if you'd like.
>> strchr_rs?
>
> Ping?
> That patch is still needed:
>
> http://marc.info/?l=openais&m=123783073728504&w=2
>
> I'll rebase it tomorrow.
Per ACK on irc, i've just committed these two changes:
>From 918bc437cd8b3af080753a861140369bf82498a3 Mon Sep 17 00:00:00 2001
From: Jim Meyering <[email protected]>
Date: Fri, 20 Mar 2009 20:39:08 +0100
Subject: [PATCH 1/2] rewrite strstr_rs not to use strdup
strstr_rs used strdup and didn't handle failure. This change removes
the use of strdup as well as the uses of strstr, since all callers
passed a string of length 1 as the second argument. This also changes
the prototype so that the 2nd parameter is a byte, not a string.
* util.h (strstr_rs): Adjust prototype.
* util.c (strstr_rs): Rewrite/simplify.
* sa-confdb.c (strstr_rs): Remove duplicate definition.
* coroparse.c (parse_section): Update callers.
---
exec/coroparse.c | 6 +++---
exec/util.c | 25 ++++---------------------
exec/util.h | 2 +-
lib/sa-confdb.c | 30 ------------------------------
4 files changed, 8 insertions(+), 55 deletions(-)
diff --git a/exec/coroparse.c b/exec/coroparse.c
index b4fd8a6..2be7a56 100644
--- a/exec/coroparse.c
+++ b/exec/coroparse.c
@@ -115,7 +115,7 @@ static int parse_section(FILE *fp,
}
/* New section ? */
- if ((loc = strstr_rs (line, "{"))) {
+ if ((loc = strstr_rs (line, '{'))) {
hdb_handle_t new_parent;
char *section = remove_whitespace(line);
@@ -128,7 +128,7 @@ static int parse_section(FILE *fp,
}
/* New key/value */
- if ((loc = strstr_rs (line, ":"))) {
+ if ((loc = strstr_rs (line, ':'))) {
char *key;
char *value;
@@ -140,7 +140,7 @@ static int parse_section(FILE *fp,
value, strlen (value) + 1);
}
- if ((loc = strstr_rs (line, "}"))) {
+ if ((loc = strstr_rs (line, '}'))) {
return 0;
}
}
diff --git a/exec/util.c b/exec/util.c
index adc4892..c51ceac 100644
--- a/exec/util.c
+++ b/exec/util.c
@@ -110,32 +110,15 @@ char *getcs_name_t (cs_name_t *name)
return ((char *)name->value);
}
-char *strstr_rs (const char *haystack, const char *needle)
+char *strstr_rs (const char *haystack, int byte)
{
- char *end_address;
- char *new_needle;
-
- new_needle = (char *)strdup (needle);
- new_needle[strlen (new_needle) - 1] = '\0';
-
- end_address = strstr (haystack, new_needle);
- if (end_address) {
- end_address += strlen (new_needle);
- end_address = strstr (end_address, needle + strlen
(new_needle));
- }
+ const char *end_address = strchr (haystack, byte);
if (end_address) {
end_address += 1; /* skip past { or = */
- do {
- if (*end_address == '\t' || *end_address == ' ') {
- end_address++;
- } else {
- break;
- }
- } while (*end_address != '\0');
+ end_address += strspn (end_address, " \t");
}
- free (new_needle);
- return (end_address);
+ return ((char *) end_address);
}
void setcs_name_t (cs_name_t *name, char *str) {
diff --git a/exec/util.h b/exec/util.h
index a03f300..77b4ac2 100644
--- a/exec/util.h
+++ b/exec/util.h
@@ -73,7 +73,7 @@ extern void _corosync_exit_error (enum e_ais_done err, const
char *file,
__attribute__((__noreturn__));
void _corosync_out_of_memory_error (void) __attribute__((__noreturn__));
extern char *getcs_name_t (cs_name_t *name);
-extern char *strstr_rs (const char *haystack, const char *needle);
+extern char *strstr_rs (const char *haystack, int byte);
extern void setcs_name_t (cs_name_t *name, char *str);
extern int cs_name_tisEqual (cs_name_t *str1, char *str2);
#endif /* UTIL_H_DEFINED */
diff --git a/lib/sa-confdb.c b/lib/sa-confdb.c
index 61ac872..2b96285 100644
--- a/lib/sa-confdb.c
+++ b/lib/sa-confdb.c
@@ -62,7 +62,6 @@ static int num_config_modules;
static struct config_iface_ver0 *config_modules[128];
void main_get_config_modules(struct config_iface_ver0 ***modules, int *num);
-char *strstr_rs (const char *haystack, const char *needle);
static int load_objdb(void)
{
@@ -149,35 +148,6 @@ void main_get_config_modules(struct config_iface_ver0
***modules, int *num)
*num = num_config_modules;
}
-/* Needed by some modules ... */
-char *strstr_rs (const char *haystack, const char *needle)
-{
- char *end_address;
- char *new_needle;
-
- new_needle = (char *)strdup (needle);
- new_needle[strlen (new_needle) - 1] = '\0';
-
- end_address = strstr (haystack, new_needle);
- if (end_address) {
- end_address += strlen (new_needle);
- end_address = strstr (end_address, needle + strlen
(new_needle));
- }
- if (end_address) {
- end_address += 1; /* skip past { or = */
- do {
- if (*end_address == '\t' || *end_address == ' ') {
- end_address++;
- } else {
- break;
- }
- } while (*end_address != '\0');
- }
-
- free (new_needle);
- return (end_address);
-}
-
int confdb_sa_init (void)
{
int res;
--
1.6.3.rc1.188.ga02b.dirty
>From d1ec0fce696bcdffb91ceac7b7d4616f4fa65d38 Mon Sep 17 00:00:00 2001
From: Jim Meyering <[email protected]>
Date: Tue, 21 Apr 2009 13:52:34 +0200
Subject: [PATCH 2/2] rename function: s/strstr_rs/strchr_rs/ to reflect new
semantics
* exec/coroparse.c (parse_section):
* exec/util.c (strchr_rs, strstr_rs):
* exec/util.h (corosync_exit_error):
---
exec/coroparse.c | 6 +++---
exec/util.c | 5 ++---
exec/util.h | 2 +-
3 files changed, 6 insertions(+), 7 deletions(-)
diff --git a/exec/coroparse.c b/exec/coroparse.c
index 2be7a56..5b87df9 100644
--- a/exec/coroparse.c
+++ b/exec/coroparse.c
@@ -115,7 +115,7 @@ static int parse_section(FILE *fp,
}
/* New section ? */
- if ((loc = strstr_rs (line, '{'))) {
+ if ((loc = strchr_rs (line, '{'))) {
hdb_handle_t new_parent;
char *section = remove_whitespace(line);
@@ -128,7 +128,7 @@ static int parse_section(FILE *fp,
}
/* New key/value */
- if ((loc = strstr_rs (line, ':'))) {
+ if ((loc = strchr_rs (line, ':'))) {
char *key;
char *value;
@@ -140,7 +140,7 @@ static int parse_section(FILE *fp,
value, strlen (value) + 1);
}
- if ((loc = strstr_rs (line, '}'))) {
+ if ((loc = strchr_rs (line, '}'))) {
return 0;
}
}
diff --git a/exec/util.c b/exec/util.c
index c51ceac..82f8070 100644
--- a/exec/util.c
+++ b/exec/util.c
@@ -1,7 +1,7 @@
/*
* Copyright (c) 2002-2004 MontaVista Software, Inc.
* Copyright (c) 2004 Open Source Development Lab
- * Copyright (c) 2006-2007 Red Hat, Inc.
+ * Copyright (c) 2006-2007, 2009 Red Hat, Inc.
*
* All rights reserved.
*
@@ -110,7 +110,7 @@ char *getcs_name_t (cs_name_t *name)
return ((char *)name->value);
}
-char *strstr_rs (const char *haystack, int byte)
+char *strchr_rs (const char *haystack, int byte)
{
const char *end_address = strchr (haystack, byte);
if (end_address) {
@@ -138,4 +138,3 @@ int cs_name_tisEqual (cs_name_t *str1, char *str2) {
return 0;
}
}
-
diff --git a/exec/util.h b/exec/util.h
index 77b4ac2..9d033d3 100644
--- a/exec/util.h
+++ b/exec/util.h
@@ -73,7 +73,7 @@ extern void _corosync_exit_error (enum e_ais_done err, const
char *file,
__attribute__((__noreturn__));
void _corosync_out_of_memory_error (void) __attribute__((__noreturn__));
extern char *getcs_name_t (cs_name_t *name);
-extern char *strstr_rs (const char *haystack, int byte);
+extern char *strchr_rs (const char *haystack, int byte);
extern void setcs_name_t (cs_name_t *name, char *str);
extern int cs_name_tisEqual (cs_name_t *str1, char *str2);
#endif /* UTIL_H_DEFINED */
--
1.6.3.rc1.188.ga02b.dirty
_______________________________________________
Openais mailing list
[email protected]
https://lists.linux-foundation.org/mailman/listinfo/openais
