ldap: fix OOM check #4467
Closed
ldap: fix OOM check #4467
Conversation
Thanks! I just find it a little bit weird to strdup() the pointer if it is NULL and check for NULL after the fact. How about instead doing something like below, to make it a little more readable? --- a/lib/ldap.c
+++ b/lib/ldap.c
@@ -845,11 +845,11 @@ static int _ldap_url_parse2(const struct connectdata *conn, LDAPURLDesc *ludp)
{
int rc = LDAP_SUCCESS;
char *path;
char *query;
char *p;
- char *q;
+ char *q = NULL;
size_t i;
if(!conn->data ||
!conn->data->state.up.path ||
conn->data->state.up.path[0] != '/' ||
@@ -863,15 +863,17 @@ static int _ldap_url_parse2(const struct connectdata *conn, LDAPURLDesc *ludp)
/* Duplicate the path */
p = path = strdup(conn->data->state.up.path + 1);
if(!path)
return LDAP_NO_MEMORY;
- /* Duplicate the query */
- q = query = strdup(conn->data->state.up.query);
- if(!query) {
- free(path);
- return LDAP_NO_MEMORY;
+ /* Duplicate the query if present */
+ if(conn->data->state.up.query) {
+ q = query = strdup(conn->data->state.up.query);
+ if(!query) {
+ free(path);
+ return LDAP_NO_MEMORY;
+ }
}
/* Parse the DN (Distinguished Name) */
if(*p) {
char *dn = p; |
Fixes #4261
You're absolutely right, thank you for the review! I force pushed the changes so as to not make it two commits. |
Thanks. I modified the commit to say this is a partial fix for #4261 which I think is two bugs, and this fixes one of them. Also I changed add init query NULL because later in cleanup there's free(query). |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Should Fix #4261
This just applies the changes described in #4261 (comment)
I tested before and after on windows 7 64 bit, it gave me the OOM message before, and the expected output after the change.