* [PATCH] avoid NULL dereference on failed malloc
@ 2009-06-14 19:46 Jim Meyering
2009-06-15 7:49 ` Michael J Gruber
0 siblings, 1 reply; 9+ messages in thread
From: Jim Meyering @ 2009-06-14 19:46 UTC (permalink / raw)
To: git list
* builtin-remote.c (get_one_entry): Use xmalloc, not malloc.
Signed-off-by: Jim Meyering <meyering@redhat.com>
---
builtin-remote.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/builtin-remote.c b/builtin-remote.c
index 709f8a6..406fb85 100644
--- a/builtin-remote.c
+++ b/builtin-remote.c
@@ -1282,7 +1282,7 @@ static int get_one_entry(struct remote *remote, void *priv)
if (remote->url_nr > 0) {
utilp = &(string_list_append(remote->name, list)->util);
- *utilp = malloc(strlen(remote->url[0])+strlen(" (fetch)")+1);
+ *utilp = xmalloc(strlen(remote->url[0])+strlen(" (fetch)")+1);
strcpy((char *) *utilp, remote->url[0]);
strcat((char *) *utilp, " (fetch)");
} else
@@ -1297,7 +1297,7 @@ static int get_one_entry(struct remote *remote, void *priv)
for (i = 0; i < url_nr; i++)
{
utilp = &(string_list_append(remote->name, list)->util);
- *utilp = malloc(strlen(url[i])+strlen(" (push)")+1);
+ *utilp = xmalloc(strlen(url[i])+strlen(" (push)")+1);
strcpy((char *) *utilp, url[i]);
strcat((char *) *utilp, " (push)");
}
--
1.6.3.2.406.gd6a466
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] avoid NULL dereference on failed malloc
2009-06-14 19:46 [PATCH] avoid NULL dereference on failed malloc Jim Meyering
@ 2009-06-15 7:49 ` Michael J Gruber
2009-06-15 20:45 ` [PATCH] builtin-remote: (get_one_entry): use strbuf Bert Wesarg
0 siblings, 1 reply; 9+ messages in thread
From: Michael J Gruber @ 2009-06-15 7:49 UTC (permalink / raw)
To: Jim Meyering; +Cc: git list, Junio C Hamano
Jim Meyering venit, vidit, dixit 14.06.2009 21:46:
>
> * builtin-remote.c (get_one_entry): Use xmalloc, not malloc.
Learning something new with every patch... Sorry, Junio; thanks, Jim!
Michael
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] builtin-remote: (get_one_entry): use strbuf
2009-06-15 7:49 ` Michael J Gruber
@ 2009-06-15 20:45 ` Bert Wesarg
2009-06-16 7:39 ` Michael J Gruber
0 siblings, 1 reply; 9+ messages in thread
From: Bert Wesarg @ 2009-06-15 20:45 UTC (permalink / raw)
To: Michael J Gruber; +Cc: Jim Meyering, git, Junio C Hamano, Bert Wesarg
Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>
---
On Mon, Jun 15, 2009 at 09:49, Michael J Gruber<git@drmicha.warpmail.net> wrote:
> Jim Meyering venit, vidit, dixit 14.06.2009 21:46:
>>
>> * builtin-remote.c (get_one_entry): Use xmalloc, not malloc.
>
> Learning something new with every patch... Sorry, Junio; thanks, Jim!
>
One more reason to re-use existing string handling functions.
Regards,
Bert
builtin-remote.c | 21 ++++++++++-----------
1 files changed, 10 insertions(+), 11 deletions(-)
diff --git a/builtin-remote.c b/builtin-remote.c
index 709f8a6..31adeaa 100644
--- a/builtin-remote.c
+++ b/builtin-remote.c
@@ -1276,17 +1276,17 @@ static int update(int argc, const char **argv)
static int get_one_entry(struct remote *remote, void *priv)
{
struct string_list *list = priv;
+ struct strbuf url_buf = STRBUF_INIT;
+ const char *url_str = NULL;
const char **url;
int i, url_nr;
- void **utilp;
if (remote->url_nr > 0) {
- utilp = &(string_list_append(remote->name, list)->util);
- *utilp = xmalloc(strlen(remote->url[0])+strlen(" (fetch)")+1);
- strcpy((char *) *utilp, remote->url[0]);
- strcat((char *) *utilp, " (fetch)");
- } else
- string_list_append(remote->name, list)->util = NULL;
+ strbuf_addf(&url_buf, "%s (fetch)", remote->url[0]);
+ url_str = strbuf_detach(&url_buf, NULL);
+ }
+ string_list_append(remote->name, list)->util = url_str;
+
if (remote->pushurl_nr) {
url = remote->pushurl;
url_nr = remote->pushurl_nr;
@@ -1296,10 +1296,9 @@ static int get_one_entry(struct remote *remote, void *priv)
}
for (i = 0; i < url_nr; i++)
{
- utilp = &(string_list_append(remote->name, list)->util);
- *utilp = xmalloc(strlen(url[i])+strlen(" (push)")+1);
- strcpy((char *) *utilp, url[i]);
- strcat((char *) *utilp, " (push)");
+ strbuf_addf(&url_buf, "%s (push)", url[i]);
+ url_str = strbuf_detach(&url_buf, NULL);
+ string_list_append(remote->name, list)->util = url_str;
}
return 0;
--
tg: (d6a466e..) bw/remote-v-use-strbuf (depends on: next)
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] builtin-remote: (get_one_entry): use strbuf
2009-06-15 20:45 ` [PATCH] builtin-remote: (get_one_entry): use strbuf Bert Wesarg
@ 2009-06-16 7:39 ` Michael J Gruber
2009-06-16 7:56 ` Bert Wesarg
0 siblings, 1 reply; 9+ messages in thread
From: Michael J Gruber @ 2009-06-16 7:39 UTC (permalink / raw)
To: Bert Wesarg; +Cc: Jim Meyering, git, Junio C Hamano
Bert Wesarg venit, vidit, dixit 15.06.2009 22:45:
> Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>
>
> ---
>
> On Mon, Jun 15, 2009 at 09:49, Michael J Gruber<git@drmicha.warpmail.net> wrote:
>> Jim Meyering venit, vidit, dixit 14.06.2009 21:46:
>>>
>>> * builtin-remote.c (get_one_entry): Use xmalloc, not malloc.
>>
>> Learning something new with every patch... Sorry, Junio; thanks, Jim!
>>
> One more reason to re-use existing string handling functions.
Well, when we discussed this before v2 I asked for guidance about
strbuf, esp. regarding the issue of allocating/freeing. From your patch
I infer that "strbuf_detach" is what I was looking for. (And yes, it is
in the api doc where I overlooked it.)
> builtin-remote.c | 21 ++++++++++-----------
> 1 files changed, 10 insertions(+), 11 deletions(-)
>
> diff --git a/builtin-remote.c b/builtin-remote.c
> index 709f8a6..31adeaa 100644
> --- a/builtin-remote.c
> +++ b/builtin-remote.c
For whatever reason, your patch does not apply (am) here on top of next
+ Jim's patch. Given the context (xmallocs), it looks like it's against
something + Jim's patch. OTOH: 709f8a6 show's a get_one_entry with
mallocs. Did you hand edit the diff?
Michael
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] builtin-remote: (get_one_entry): use strbuf
2009-06-16 7:39 ` Michael J Gruber
@ 2009-06-16 7:56 ` Bert Wesarg
2009-06-16 10:49 ` Michael J Gruber
0 siblings, 1 reply; 9+ messages in thread
From: Bert Wesarg @ 2009-06-16 7:56 UTC (permalink / raw)
To: Michael J Gruber; +Cc: Jim Meyering, git, Junio C Hamano
On Tue, Jun 16, 2009 at 09:39, Michael J Gruber<git@drmicha.warpmail.net> wrote:
> Bert Wesarg venit, vidit, dixit 15.06.2009 22:45:
>> Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>
>>
>> ---
>>
>> On Mon, Jun 15, 2009 at 09:49, Michael J Gruber<git@drmicha.warpmail.net> wrote:
>>> Jim Meyering venit, vidit, dixit 14.06.2009 21:46:
>>>>
>>>> * builtin-remote.c (get_one_entry): Use xmalloc, not malloc.
>>>
>>> Learning something new with every patch... Sorry, Junio; thanks, Jim!
>>>
>> One more reason to re-use existing string handling functions.
>
> Well, when we discussed this before v2 I asked for guidance about
> strbuf, esp. regarding the issue of allocating/freeing.
Well, I stopped reading after this question of yours: "But what do
strbufs bring us here?"
> From your patch
> I infer that "strbuf_detach" is what I was looking for. (And yes, it is
> in the api doc where I overlooked it.)
>
>> builtin-remote.c | 21 ++++++++++-----------
>> 1 files changed, 10 insertions(+), 11 deletions(-)
>>
>> diff --git a/builtin-remote.c b/builtin-remote.c
>> index 709f8a6..31adeaa 100644
>> --- a/builtin-remote.c
>> +++ b/builtin-remote.c
>
> For whatever reason, your patch does not apply (am) here on top of next
> + Jim's patch. Given the context (xmallocs), it looks like it's against
> something + Jim's patch. OTOH: 709f8a6 show's a get_one_entry with
> mallocs. Did you hand edit the diff?
Its on top of next (d6a466e528119011d512379f7f9dfac26deb7fd9), plus
hand editing s/malloc/xmalloc/g.
Sorry for this.
Bert
>
> Michael
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] builtin-remote: (get_one_entry): use strbuf
2009-06-16 7:56 ` Bert Wesarg
@ 2009-06-16 10:49 ` Michael J Gruber
2009-06-22 8:24 ` Bert Wesarg
0 siblings, 1 reply; 9+ messages in thread
From: Michael J Gruber @ 2009-06-16 10:49 UTC (permalink / raw)
To: Bert Wesarg; +Cc: Jim Meyering, git, Junio C Hamano
Bert Wesarg venit, vidit, dixit 16.06.2009 09:56:
> On Tue, Jun 16, 2009 at 09:39, Michael J Gruber<git@drmicha.warpmail.net> wrote:
>> Bert Wesarg venit, vidit, dixit 15.06.2009 22:45:
>>> Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>
>>>
>>> ---
>>>
>>> On Mon, Jun 15, 2009 at 09:49, Michael J Gruber<git@drmicha.warpmail.net> wrote:
>>>> Jim Meyering venit, vidit, dixit 14.06.2009 21:46:
>>>>>
>>>>> * builtin-remote.c (get_one_entry): Use xmalloc, not malloc.
>>>>
>>>> Learning something new with every patch... Sorry, Junio; thanks, Jim!
>>>>
>>> One more reason to re-use existing string handling functions.
>>
>> Well, when we discussed this before v2 I asked for guidance about
>> strbuf, esp. regarding the issue of allocating/freeing.
> Well, I stopped reading after this question of yours: "But what do
> strbufs bring us here?"
Well, as I said I was strbuf-agnostic - some questions are really meant
to be questions ;)
In any case, I've learned from your patch.
>
>> From your patch
>> I infer that "strbuf_detach" is what I was looking for. (And yes, it is
>> in the api doc where I overlooked it.)
>>
>>> builtin-remote.c | 21 ++++++++++-----------
>>> 1 files changed, 10 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/builtin-remote.c b/builtin-remote.c
>>> index 709f8a6..31adeaa 100644
>>> --- a/builtin-remote.c
>>> +++ b/builtin-remote.c
>>
>> For whatever reason, your patch does not apply (am) here on top of next
>> + Jim's patch. Given the context (xmallocs), it looks like it's against
>> something + Jim's patch. OTOH: 709f8a6 show's a get_one_entry with
>> mallocs. Did you hand edit the diff?
> Its on top of next (d6a466e528119011d512379f7f9dfac26deb7fd9), plus
> hand editing s/malloc/xmalloc/g.
> Sorry for this.
>
> Bert
Junio will have to deal with it...
Michael
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] builtin-remote: (get_one_entry): use strbuf
2009-06-16 10:49 ` Michael J Gruber
@ 2009-06-22 8:24 ` Bert Wesarg
2009-06-22 21:32 ` Jeff King
0 siblings, 1 reply; 9+ messages in thread
From: Bert Wesarg @ 2009-06-22 8:24 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Michael J Gruber, Jim Meyering, git, Bert Wesarg
Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>
---
On Tue, Jun 16, 2009 at 12:49, Michael J Gruber<git@drmicha.warpmail.net> wrote:
> Bert Wesarg venit, vidit, dixit 16.06.2009 09:56:
>> On Tue, Jun 16, 2009 at 09:39, Michael J Gruber<git@drmicha.warpmail.net> wrote:
>>> For whatever reason, your patch does not apply (am) here on top of next
>>> + Jim's patch. Given the context (xmallocs), it looks like it's against
>>> something + Jim's patch. OTOH: 709f8a6 show's a get_one_entry with
>>> mallocs. Did you hand edit the diff?
>> Its on top of next (d6a466e528119011d512379f7f9dfac26deb7fd9), plus
>> hand editing s/malloc/xmalloc/g.
>> Sorry for this.
>>
>> Bert
>
> Junio will have to deal with it...
Here is an updated version.
Bert
builtin-remote.c | 21 ++++++++++-----------
1 files changed, 10 insertions(+), 11 deletions(-)
diff --git a/builtin-remote.c b/builtin-remote.c
index 658d578..d47a124 100644
--- a/builtin-remote.c
+++ b/builtin-remote.c
@@ -1276,17 +1276,17 @@ static int update(int argc, const char **argv)
static int get_one_entry(struct remote *remote, void *priv)
{
struct string_list *list = priv;
+ struct strbuf url_buf = STRBUF_INIT;
+ const char *url_str = NULL;
const char **url;
int i, url_nr;
- void **utilp;
if (remote->url_nr > 0) {
- utilp = &(string_list_append(remote->name, list)->util);
- *utilp = xmalloc(strlen(remote->url[0])+strlen(" (fetch)")+1);
- strcpy((char *) *utilp, remote->url[0]);
- strcat((char *) *utilp, " (fetch)");
- } else
- string_list_append(remote->name, list)->util = NULL;
+ strbuf_addf(&url_buf, "%s (fetch)", remote->url[0]);
+ url_str = strbuf_detach(&url_buf, NULL);
+ }
+ string_list_append(remote->name, list)->util = url_str;
+
if (remote->pushurl_nr) {
url = remote->pushurl;
url_nr = remote->pushurl_nr;
@@ -1296,10 +1296,9 @@ static int get_one_entry(struct remote *remote, void *priv)
}
for (i = 0; i < url_nr; i++)
{
- utilp = &(string_list_append(remote->name, list)->util);
- *utilp = xmalloc(strlen(url[i])+strlen(" (push)")+1);
- strcpy((char *) *utilp, url[i]);
- strcat((char *) *utilp, " (push)");
+ strbuf_addf(&url_buf, "%s (push)", url[i]);
+ url_str = strbuf_detach(&url_buf, NULL);
+ string_list_append(remote->name, list)->util = url_str;
}
return 0;
--
tg: (363bdbe..) bw/remote-v-use-strbuf (depends on: next)
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] builtin-remote: (get_one_entry): use strbuf
2009-06-22 8:24 ` Bert Wesarg
@ 2009-06-22 21:32 ` Jeff King
2009-06-22 22:27 ` [PATCH v2] " Bert Wesarg
0 siblings, 1 reply; 9+ messages in thread
From: Jeff King @ 2009-06-22 21:32 UTC (permalink / raw)
To: Bert Wesarg; +Cc: Junio C Hamano, Michael J Gruber, Jim Meyering, git
On Mon, Jun 22, 2009 at 10:24:19AM +0200, Bert Wesarg wrote:
> --- a/builtin-remote.c
> +++ b/builtin-remote.c
> @@ -1276,17 +1276,17 @@ static int update(int argc, const char **argv)
> static int get_one_entry(struct remote *remote, void *priv)
> {
> struct string_list *list = priv;
> + struct strbuf url_buf = STRBUF_INIT;
> + const char *url_str = NULL;
> const char **url;
> int i, url_nr;
> - void **utilp;
>
> if (remote->url_nr > 0) {
> - utilp = &(string_list_append(remote->name, list)->util);
> - *utilp = xmalloc(strlen(remote->url[0])+strlen(" (fetch)")+1);
> - strcpy((char *) *utilp, remote->url[0]);
> - strcat((char *) *utilp, " (fetch)");
> - } else
> - string_list_append(remote->name, list)->util = NULL;
> + strbuf_addf(&url_buf, "%s (fetch)", remote->url[0]);
> + url_str = strbuf_detach(&url_buf, NULL);
> + }
> + string_list_append(remote->name, list)->util = url_str;
> +
This causes const warnings due to the 'const' on url_str. One solution
is just s/const char/char/.
However, I think it is actually more readable to do away with url_str
entirely. The original if/else logic was much easier to follow than
realizing that url_str is initialized to NULL, and then never changed.
IOW:
if (remote->url_nr > 0) {
strbuf_addf(&url_buf, "%s (fetch)", remote->url[0]);
string_list_append(remote->name, list)->util =
strbuf_detach(&url_buf, NULL);
}
else
string_list_append(remote->name, list)->util = NULL;
> @@ -1296,10 +1296,9 @@ static int get_one_entry(struct remote *remote, void *priv)
> }
> for (i = 0; i < url_nr; i++)
> {
> - utilp = &(string_list_append(remote->name, list)->util);
> - *utilp = xmalloc(strlen(url[i])+strlen(" (push)")+1);
> - strcpy((char *) *utilp, url[i]);
> - strcat((char *) *utilp, " (push)");
> + strbuf_addf(&url_buf, "%s (push)", url[i]);
> + url_str = strbuf_detach(&url_buf, NULL);
> + string_list_append(remote->name, list)->util = url_str;
> }
And then this one is re-using url_str for a totally unrelated thing,
but could just be:
string_list_append(remote->name, list)->util =
strbuf_detach(&url_buf, NULL);
But that is somewhat nit-picking. As long as the const-warning goes
away, I will be happy enough.
-Peff
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2] builtin-remote: (get_one_entry): use strbuf
2009-06-22 21:32 ` Jeff King
@ 2009-06-22 22:27 ` Bert Wesarg
0 siblings, 0 replies; 9+ messages in thread
From: Bert Wesarg @ 2009-06-22 22:27 UTC (permalink / raw)
To: Jeff King
Cc: Junio C Hamano, Michael J Gruber, Jim Meyering, git, Bert Wesarg
Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>
---
v2: - remove assigment indirection
- keep old code flow
On Mon, Jun 22, 2009 at 23:32, Jeff King <peff@peff.net> wrote:
> But that is somewhat nit-picking. As long as the const-warning goes
> away, I will be happy enough.
>
Thanks for the review. I have no objections to your comments and have all
incroporated into v2.
Thanks,
Bert
> -Peff
builtin-remote.c | 16 +++++++---------
1 files changed, 7 insertions(+), 9 deletions(-)
diff --git a/builtin-remote.c b/builtin-remote.c
index 658d578..2fb76d3 100644
--- a/builtin-remote.c
+++ b/builtin-remote.c
@@ -1276,15 +1276,14 @@ static int update(int argc, const char **argv)
static int get_one_entry(struct remote *remote, void *priv)
{
struct string_list *list = priv;
+ struct strbuf url_buf = STRBUF_INIT;
const char **url;
int i, url_nr;
- void **utilp;
if (remote->url_nr > 0) {
- utilp = &(string_list_append(remote->name, list)->util);
- *utilp = xmalloc(strlen(remote->url[0])+strlen(" (fetch)")+1);
- strcpy((char *) *utilp, remote->url[0]);
- strcat((char *) *utilp, " (fetch)");
+ strbuf_addf(&url_buf, "%s (fetch)", remote->url[0]);
+ string_list_append(remote->name, list)->util =
+ strbuf_detach(&url_buf, NULL);
} else
string_list_append(remote->name, list)->util = NULL;
if (remote->pushurl_nr) {
@@ -1296,10 +1295,9 @@ static int get_one_entry(struct remote *remote, void *priv)
}
for (i = 0; i < url_nr; i++)
{
- utilp = &(string_list_append(remote->name, list)->util);
- *utilp = xmalloc(strlen(url[i])+strlen(" (push)")+1);
- strcpy((char *) *utilp, url[i]);
- strcat((char *) *utilp, " (push)");
+ strbuf_addf(&url_buf, "%s (push)", url[i]);
+ string_list_append(remote->name, list)->util =
+ strbuf_detach(&url_buf, NULL);
}
return 0;
--
tg: (d4b46b0..) bw/remote-v-use-strbuf (depends on: next)
^ permalink raw reply related [flat|nested] 9+ messages in thread
end of thread, other threads:[~2009-06-22 22:28 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-06-14 19:46 [PATCH] avoid NULL dereference on failed malloc Jim Meyering
2009-06-15 7:49 ` Michael J Gruber
2009-06-15 20:45 ` [PATCH] builtin-remote: (get_one_entry): use strbuf Bert Wesarg
2009-06-16 7:39 ` Michael J Gruber
2009-06-16 7:56 ` Bert Wesarg
2009-06-16 10:49 ` Michael J Gruber
2009-06-22 8:24 ` Bert Wesarg
2009-06-22 21:32 ` Jeff King
2009-06-22 22:27 ` [PATCH v2] " Bert Wesarg
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.