tsearch.3: dispose properly of allocated resources in example code

PATCH In review André Goddard Rosa 2 participants 8 messages Sep 21, 2008 – Sep 25, 2008 5 messages not in the archive Man-pages: manual pages for Linux -- Sections 2, 3, 4, 5, and 7

lore

Share & tools

lore

André Goddard Rosa

author

2008-09-21 06:10 UTC #0 lore raw

PATCH 1/1 tsearch.3: dispose properly of allocated resources in example code

As there are many users who base their own code on these examples, we should avoid leaking memory when using functions of tsearch() family. In the provided example, tsearch() leaks memory whenever it tries to insert a number which was already inserted before.

Please CC me, as I´m not subscribed.

Applies to man-pages-3.09.

tsearch.3.goddard renamed from tsearch.3 +3 -0

@@ -246,8 +246,11 @@
         val = tsearch((void *) ptr, &root, compare);
         if (val == NULL)
             exit(EXIT_FAILURE);
+        else if (!((*(int **) val) == ptr))
+            free(ptr);
     }
     twalk(root, action);
+    tdestroy(root, free);
     exit(EXIT_SUCCESS);
 }
 .fi

tsearch.3.goddard renamed from tsearch.3 +3 -0

@@ -246,8 +246,11 @@
         val = tsearch((void *) ptr, &root, compare);
         if (val == NULL)
             exit(EXIT_FAILURE);
+        else if (!((*(int **) val) == ptr))
+            free(ptr);
     }
     twalk(root, action);
+    tdestroy(root, free);
     exit(EXIT_SUCCESS);
 }
 .fi

A message that is not in the archive (sent off-list or not yet received): 200809210310.43647.andre.goddard-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org

Michael Kerrisk

2008-09-23 06:28 UTC #2 lore raw

André,

Quoted 4 lines · André Goddard Rosa

As there are many users who base their own code on these examples, we should avoid leaking memory when using functions of tsearch() family. In the provided example, tsearch() leaks memory whenever it tries to insert a number which was already inserted before.

Thanks. My first patch applied via 'git am'!

I applied this patch for man-pages-3.10. (I added #define _GNU_SOURCE
since that's required to expose the tdestroy() declaration.)

Cheers,

Michael

Quoted 16 lines

tsearch.3.goddard 2008-09-21 02:42:55.000000000 -0300 renamed from tsearch.3 2008-09-10 10:16:13.000000000 -0300 +3 -0

@@ -246,8 +246,11 @@
        val = tsearch((void *) ptr, &root, compare);
        if (val == NULL)
            exit(EXIT_FAILURE);
+        else if (!((*(int **) val) == ptr))
+            free(ptr);
    }
    twalk(root, action);
+    tdestroy(root, free);
    exit(EXIT_SUCCESS);
 }
 .fi

Signature · 8 lines

Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
man-pages online: http://www.kernel.org/doc/man-pages/online_pages.html
Found a bug? http://www.kernel.org/doc/man-pages/reporting_bugs.html
--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): cfd18e0f0809222328r599e22c1je0f4261ee969bca8-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org

André Goddard Rosa

author

2008-09-23 13:23 UTC #4 lore raw

Quoted 9 lines · Michael Kerrisk

André,
Quoted 4 lines · André Goddard Rosa
As there are many users who base their own code on these examples, we should avoid leaking memory when using functions of tsearch() family. In the provided example, tsearch() leaks memory whenever it tries to insert a number which was already inserted before.

Quoted 6 lines

Thanks. My first patch applied via 'git am'!
I applied this patch for man-pages-3.10. (I added #define _GNU_SOURCE
since that's required to expose the tdestroy() declaration.)

Hi, Michael!

Very nice, thank you! Good to know that you're using git.

I appreciate your work in the man pages, they were improved a lot
since you've started working on them. Also, I appreciate your concerns regarding paccept().

Notice that I sent two different patches in a sequence (sorry for that).

This is the first one:

Quoted 1 line

+ else if (!((*(int **) val) == ptr))

and this is the second one:

+ else if ((*(int **) val) != ptr)

They work the same, but second is cleaner and simpler ;-)

Kind regards,

Signature · 6 lines

[]s,
André Goddard
--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): b8bf37780809230623k62d43d6cgf11af4820566cd2d-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org

Michael Kerrisk

2008-09-23 13:56 UTC #6 lore raw

André,

Quoted 13 lines · André Goddard Rosa

Quoted 11 lines · Michael Kerrisk
André,
Quoted 4 lines · André Goddard Rosa
As there are many users who base their own code on these examples, we should avoid leaking memory when using functions of tsearch() family. In the provided example, tsearch() leaks memory whenever it tries to insert a number which was already inserted before.
Thanks. My first patch applied via 'git am'!

Quoted 8 lines

Quoted 4 lines
I applied this patch for man-pages-3.10. (I added #define _GNU_SOURCE
since that's required to expose the tdestroy() declaration.)
Hi, Michael!
Very nice, thank you! Good to know that you're using git.

(At last... '-).)

Quoted 2 lines

I appreciate your work in the man pages, they were improved a lot
since you've started working on them.

Thanks!

Quoted 4 lines

Also, I appreciate your concerns
regarding paccept().
Notice that I sent two different patches in a sequence (sorry for that).

It's a good thing you mentioned this, since I did _not_ notice that:
you sent two mails at almost the same, with the same description, and which looked otherwise identical (unless one looked very close), so I assumed you had accidentally sent the same mail twice. (Best to change the description a little so I know you are resending a different patch.)

Quoted 9 lines

This is the first one:
Quoted 1 line
+ else if (!((*(int **) val) == ptr))
and this is the second one:
+ else if ((*(int **) val) != ptr)
They work the same, but second is cleaner and simpler ;-)

Okay -- I've fixed the page to use the second version. And you only just got that one in in time for 3.10!

Thanks,

Michael
--
To unsubscribe from this list: send the line "unsubscribe linux-man" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

André Goddard Rosa

author

2008-09-24 14:01 UTC #7 lore raw

PATCH 1/1 hsearch.3: Call hdestroy() after using hash table created by hcreate(), for the sake of completeness

In reply to a message not in the archive

From dc0f26d3112519229671c621832f87a48e972642 Mon Sep 17 00:00:00 2001
From: =?utf-8?q?Andr=C3=A9=20Goddard=20Rosa?= <[email protected]>
Date: Wed, 24 Sep 2008 10:59:37 -0300
Subject: [PATCH] Call hdestroy() after using hash table created by hcreate(), for the sake of completeness.
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit

Signed-off-by André Goddard Rosa <[email protected]>

---
 man3/hsearch.3 |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

man3/hsearch.3 +1 -0

index 16e3b6b..d5ae21c 100644
@@ -325,6 +325,7 @@ main(void)
         printf("%9.9s \-> %9.9s:%d\\n", e.key,
                ep ? ep\->key : "NULL", ep ? (int)(ep\->data) : 0);
     }
+    hdestroy();
     exit(EXIT_SUCCESS);
 }
 .fi

Signature · 3 lines

1.6.0.2.508.g24593

Signed-off-by André Goddard Rosa <[email protected]>

---
 man3/hsearch.3 |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

man3/hsearch.3 +1 -0

index 16e3b6b..d5ae21c 100644
@@ -325,6 +325,7 @@ main(void)
         printf("%9.9s \-> %9.9s:%d\\n", e.key,
                ep ? ep\->key : "NULL", ep ? (int)(ep\->data) : 0);
     }
+    hdestroy();
     exit(EXIT_SUCCESS);
 }
 .fi

Signature · 1 line

1.6.0.2.508.g24593

A message that is not in the archive (sent off-list or not yet received): 200809241101.23128.andre.goddard-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org

Michael Kerrisk

2008-09-25 08:04 UTC #9 lore raw

Re: [patch] hsearch.3: Call hdestroy() after using hash table created by hcreate(), for the sake of completeness

Andre,

Quoted 18 lines · André Goddard Rosa

From dc0f26d3112519229671c621832f87a48e972642 Mon Sep 17 00:00:00 2001
From: =?utf-8?q?Andr=C3=A9=20Goddard=20Rosa?= <[email protected]>
Date: Wed, 24 Sep 2008 10:59:37 -0300
Subject: [PATCH] Call hdestroy() after using hash table created by hcreate(), for the sake of completeness.
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit
Signed-off-by André Goddard Rosa <[email protected]>
---
 man3/hsearch.3 |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
man3/hsearch.3 +0 -0
index 16e3b6b..d5ae21c 100644

man3/hsearch.3 main(void) · line 331

Quoted 7 lines

man3/hsearch.3 +1 -0

@@ -325,6 +325,7 @@ main(void)
        printf("%9.9s \-> %9.9s:%d\\n", e.key,
               ep ? ep\->key : "NULL", ep ? (int)(ep\->data) : 0);
    }
+    hdestroy();
    exit(EXIT_SUCCESS);
 }
 .fi

Yes, good idea. Thanks. Applied for 3.11.

Cheers,

Michael

Signature · 8 lines

Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
man-pages online: http://www.kernel.org/doc/man-pages/online_pages.html
Found a bug? http://www.kernel.org/doc/man-pages/reporting_bugs.html
--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

André Goddard Rosa

author

2008-09-24 14:01 UTC #10 lore raw

PATCH 1/1 strcpy.3: Avoid redundantly filling the position 'n - 1' two times in strncpy() example code

In reply to a message not in the archive

From 88a02845c606413444bfb055935303b4896d0271 Mon Sep 17 00:00:00 2001
From: =?utf-8?q?Andr=C3=A9=20Goddard=20Rosa?= <[email protected]>
Date: Wed, 24 Sep 2008 10:25:08 -0300
Subject: [PATCH] Avoid redundantly filling the position 'n - 1' two times in strncpy() example code.
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit

When we use 'strncpy(buf, str, n)', strncpy() will fill chars from 0..(n - 1).
As we force the character at position 'n - 1' to be '\0' anyway, don't fill it in strncpy() before.

Signed-off-by André Goddard Rosa <[email protected]>

---
 man3/strcpy.3 |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

man3/strcpy.3 +1 -1

index 7a8091f..d2e08a3 100644
@@ -115,7 +115,7 @@ as follows:
 .in +4n
 .nf
 
-strncpy(buf, str, n);
+strncpy(buf, str, n \- 1);
 if (n > 0)
     buf[n \- 1]= \(aq\\0\(aq;
 .fi

Signature · 3 lines

1.6.0.2.508.g24593

When we use 'strncpy(buf, str, n)', strncpy() will fill chars from 0..(n - 1).
As we force the character at position 'n - 1' to be '\0' anyway, don't fill it in strncpy() before.

Signed-off-by André Goddard Rosa <[email protected]>

---
 man3/strcpy.3 |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

man3/strcpy.3 +1 -1

index 7a8091f..d2e08a3 100644
@@ -115,7 +115,7 @@ as follows:
 .in +4n
 .nf
 
-strncpy(buf, str, n);
+strncpy(buf, str, n \- 1);
 if (n > 0)
     buf[n \- 1]= \(aq\\0\(aq;
 .fi

Signature · 1 line

1.6.0.2.508.g24593

A message that is not in the archive (sent off-list or not yet received): 200809241101.34334.andre.goddard-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org

Michael Kerrisk

2008-09-25 08:14 UTC #12 lore raw

Re: [patch] strcpy.3: Avoid redundantly filling the position 'n - 1' two times in strncpy() example code

Hi Andre,

Quoted 10 lines · André Goddard Rosa

From 88a02845c606413444bfb055935303b4896d0271 Mon Sep 17 00:00:00 2001
From: =?utf-8?q?Andr=C3=A9=20Goddard=20Rosa?= <[email protected]>
Date: Wed, 24 Sep 2008 10:25:08 -0300
Subject: [PATCH] Avoid redundantly filling the position 'n - 1' two times in strncpy() example code.
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit
When we use 'strncpy(buf, str, n)', strncpy() will fill chars from 0..(n - 1).
As we force the character at position 'n - 1' to be '\0' anyway, don't fill it in strncpy() before.

Thanks! Applied for 3.11.

Cheers,

Michael

Quoted 23 lines

Signed-off-by André Goddard Rosa <[email protected]>

---
 man3/strcpy.3 |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

man3/strcpy.3 +1 -2

index 7a8091f..d2e08a3 100644
@@ -115,7 +115,7 @@ as follows:
 .in +4n
 .nf
 
-strncpy(buf, str, n);
+strncpy(buf, str, n \- 1);
 if (n > 0)
    buf[n \- 1]= \(aq\\0\(aq;
 .fi
--

1.6.0.2.508.g24593

Signature · 8 lines

Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
man-pages online: http://www.kernel.org/doc/man-pages/online_pages.html
Found a bug? http://www.kernel.org/doc/man-pages/reporting_bugs.html
--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html