[PATCH] Remove a bunch of useless if NULL before free
Hi, I've removed a lot of "if(p) free(p)" form in MPlayer files. There are 33 files modified (from root, sub and stream directories). I hope this patch is fine. If one single patch is not good, I've also the patch split in 33 files. Regards, -- Clément B. Not sent from a jesusPhone.
On Thu, Nov 04, 2010 at 04:06:09AM +0100, Clément Bœsch wrote:
I've removed a lot of "if(p) free(p)" form in MPlayer files. There are 33 files modified (from root, sub and stream directories). I hope this patch is fine. If one single patch is not good, I've also the patch split in 33 files.
- if(entries[num].title) { - // When we have info in playtree we add this info - free(entries[num].title); - } - if(entries[num].length) { - // When we have info in playtree we add this info - free(entries[num].length); - } + free(entries[num].title); // When we have info in playtree we add this info + free(entries[num].length); // When we have info in playtree we add this info
I don't like that you moved the comment, resulting in overlong lines. That is of course besides the fact that it is twice the same comment and both are incomprehensible to me which is not your fault. But otherwise I guess it's ok.
On Thu, Nov 04, 2010 at 07:54:11AM +0100, Reimar Döffinger wrote:
On Thu, Nov 04, 2010 at 04:06:09AM +0100, Clément Bœsch wrote:
I've removed a lot of "if(p) free(p)" form in MPlayer files. There are 33 files modified (from root, sub and stream directories). I hope this patch is fine. If one single patch is not good, I've also the patch split in 33 files.
- if(entries[num].title) { - // When we have info in playtree we add this info - free(entries[num].title); - } - if(entries[num].length) { - // When we have info in playtree we add this info - free(entries[num].length); - } + free(entries[num].title); // When we have info in playtree we add this info + free(entries[num].length); // When we have info in playtree we add this info
I don't like that you moved the comment, resulting in overlong lines. That is of course besides the fact that it is twice the same comment and both are incomprehensible to me which is not your fault. But otherwise I guess it's ok.
What do you propose? Removing them? Just place it one time before the 2 calls to free? -- Clément B. Not sent from a jesusPhone.
On Thu, Nov 04, 2010 at 07:54:11AM +0100, Reimar Döffinger wrote:
On Thu, Nov 04, 2010 at 04:06:09AM +0100, Clément Bœsch wrote:
I've removed a lot of "if(p) free(p)" form in MPlayer files. There are 33 files modified (from root, sub and stream directories). I hope this patch is fine. If one single patch is not good, I've also the patch split in 33 files.
- if(entries[num].title) { - // When we have info in playtree we add this info - free(entries[num].title); - } - if(entries[num].length) { - // When we have info in playtree we add this info - free(entries[num].length); - } + free(entries[num].title); // When we have info in playtree we add this info + free(entries[num].length); // When we have info in playtree we add this info
I don't like that you moved the comment, resulting in overlong lines. That is of course besides the fact that it is twice the same comment and both are incomprehensible to me which is not your fault. But otherwise I guess it's ok.
Is this version ok with you? -- Clément B. Not sent from a jesusPhone.
On Thu, Nov 04, 2010 at 10:01:46PM +0100, Clément Bœsch wrote:
On Thu, Nov 04, 2010 at 07:54:11AM +0100, Reimar Döffinger wrote:
On Thu, Nov 04, 2010 at 04:06:09AM +0100, Clément Bœsch wrote:
I've removed a lot of "if(p) free(p)" form in MPlayer files. There are 33 files modified (from root, sub and stream directories). I hope this patch is fine. If one single patch is not good, I've also the patch split in 33 files.
- if(entries[num].title) { - // When we have info in playtree we add this info - free(entries[num].title); - } - if(entries[num].length) { - // When we have info in playtree we add this info - free(entries[num].length); - } + free(entries[num].title); // When we have info in playtree we add this info + free(entries[num].length); // When we have info in playtree we add this info
I don't like that you moved the comment, resulting in overlong lines. That is of course besides the fact that it is twice the same comment and both are incomprehensible to me which is not your fault. But otherwise I guess it's ok.
Is this version ok with you?
Yes, fine.
On Thu, Nov 04, 2010 at 10:01:46PM +0100, Clément Bœsch wrote:
Is this version ok with you?
--- a/stream/tvi_v4l.c +++ b/stream/tvi_v4l.c @@ -702,10 +700,8 @@ static int uninit(priv_t *priv)
- if(priv->vbi_dev){ - free(priv->vbi_dev); - priv->vbi_dev=0; - } + free(priv->vbi_dev); + priv->vbi_dev = NULL;
Was this 0 --> NULL change done on purpose? Anyway, patch applied. Diego
On Sun, Nov 07, 2010 at 01:45:31PM +0100, Diego Biurrun wrote:
On Thu, Nov 04, 2010 at 10:01:46PM +0100, Clément Bœsch wrote:
Is this version ok with you?
--- a/stream/tvi_v4l.c +++ b/stream/tvi_v4l.c @@ -702,10 +700,8 @@ static int uninit(priv_t *priv)
- if(priv->vbi_dev){ - free(priv->vbi_dev); - priv->vbi_dev=0; - } + free(priv->vbi_dev); + priv->vbi_dev = NULL;
Was this 0 --> NULL change done on purpose?
I'm pretty sure it was. Strictly speaking it cosmetic only, but to a degree so is the whole patch.
On Sun, Nov 07, 2010 at 05:55:40PM +0100, Reimar Döffinger wrote:
On Sun, Nov 07, 2010 at 01:45:31PM +0100, Diego Biurrun wrote:
On Thu, Nov 04, 2010 at 10:01:46PM +0100, Clément Bœsch wrote:
Is this version ok with you?
--- a/stream/tvi_v4l.c +++ b/stream/tvi_v4l.c @@ -702,10 +700,8 @@ static int uninit(priv_t *priv)
- if(priv->vbi_dev){ - free(priv->vbi_dev); - priv->vbi_dev=0; - } + free(priv->vbi_dev); + priv->vbi_dev = NULL;
Was this 0 --> NULL change done on purpose?
I'm pretty sure it was. Strictly speaking it cosmetic only, but to a degree so is the whole patch.
Yes sorry about that, but since the line needed to be moved, i thought making it much more explicit about the fact it was a pointer wasn't harmless. -- Clément B. Not sent from a jesusPhone.
participants (3)
-
Clément Bœsch -
Diego Biurrun -
Reimar Döffinger