Re: [hackers] [st] Add xmalloc and xrealloc wrappers || "Roberto E. Vargas Caballero"

From: Joerg Zinke <mail_AT_umaxx.net>
Date: Thu, 13 Sep 2012 16:37:51 +0200

This xrealloc() is a good a example for bad c programming.
Quoting the OpenBSD realloc(3) manpage:

<snip>
When using realloc() be careful to avoid the following idiom:

           size += 50;
           if ((p = realloc(p, size)) == NULL)
                   return (NULL);

     Do not adjust the variable describing how much memory has been allocated
     until the allocation has been successful. This can cause aberrant
     program behavior if the incorrect size value is used. In most cases, the
     above sample will also result in a leak of memory. As stated earlier, a
     return value of NULL indicates that the old object still remains
     allocated. Better code looks like this:

           newsize = size + 50;
           if ((newp = realloc(p, newsize)) == NULL) {
                   free(p);
                   p = NULL;
                   size = 0;
                   return (NULL);
           }
           p = newp;
           size = newsize;

     As with malloc() it is important to ensure the new size value will not
     overflow; i.e. avoid allocations like the following:

           if ((newp = realloc(p, num * size)) == NULL) {
                   ...
<snap>

You may want to have a look into the OpenSSH source for better
implementations:
http://www.openbsd.org/cgi-bin/cvsweb/src/usr.bin/ssh/xmalloc.c?rev=1.16.18.1;content-type=text%2Fplain

Am 12.09.2012 um 21:27 schrieb hg_AT_suckless.org:

> changeset: 295:2a91ba0148f3
> tag: tip
> user: "Roberto E. Vargas Caballero" <k0ga_AT_shike2.com>
> date: Wed Sep 12 21:25:35 2012 +0200
> files: st.c
> description:
> Add xmalloc and xrealloc wrappers
> If malloc or realloc fail they return NULL. Theorically this condition
> should be tested in the code, but it's a strange condition today (basically
> if this is hapenning thenyou have a big problem), and even Linux never returns
> NULL in the default configuration (only if the process don't have room in
> the space address, something a bit impossible in the case of st). But stis
> enough small for being executed in low resources computers where this can be
> a real problem. So the easy way is creating a wrappers function for them and
> call to die in case of error.
> ---
> st.c | 44 +++++++++++++++++++++++++++++++-------------
> 1 file changed, 31 insertions(+), 13 deletions(-)
>
>
> diff -r 1d5a3f61c6f1 -r 2a91ba0148f3 st.c
> --- a/st.c Wed Sep 12 13:20:10 2012 +0200
> +++ b/st.c Wed Sep 12 21:25:35 2012 +0200
> _AT_@ -326,6 +326,9 @@
> static int utf8size(char *);
> static int isfullutf8(char *, int);
>
> +static void *xmalloc(size_t);
> +static void *xrealloc(void *, size_t);
> +
> static void (*handler[LASTEvent])(XEvent *) = {
> [KeyPress] = kpress,
> [ClientMessage] = cmessage,
> _AT_@ -359,6 +362,21 @@
> static char *opt_embed = NULL;
> static char *opt_class = NULL;
>
> +void *
> +xmalloc(size_t len) {
> + void *p = malloc(len);
> + if(!p)
> + die("Out of memory");
> + return p;
> +}
> +
> +void *
> +xrealloc(void *p, size_t len) {
> + if((p = realloc(p, len)) == NULL)
> + die("Out of memory");
> + return p;
> +}
> +
> int
> utf8decode(char *s, long *u) {
> uchar c;
> _AT_@ -565,7 +583,7 @@
>
> else {
> bufsize = (term.col+1) * (sel.e.y-sel.b.y+1) * UTF_SIZ;
> - ptr = str = malloc(bufsize);
> + ptr = str = xmalloc(bufsize);
>
> /* append every set & selected glyph to the selection */
> for(y = 0; y < term.row; y++) {
> _AT_@ -918,14 +936,14 @@
> tnew(int col, int row) {
> /* set screen size */
> term.row = row, term.col = col;
> - term.line = malloc(term.row * sizeof(Line));
> - term.alt = malloc(term.row * sizeof(Line));
> - term.dirty = malloc(term.row * sizeof(*term.dirty));
> - term.tabs = malloc(term.col * sizeof(*term.tabs));
> + term.line = xmalloc(term.row * sizeof(Line));
> + term.alt = xmalloc(term.row * sizeof(Line));
> + term.dirty = xmalloc(term.row * sizeof(*term.dirty));
> + term.tabs = xmalloc(term.col * sizeof(*term.tabs));
>
> for(row = 0; row < term.row; row++) {
> - term.line[row] = malloc(term.col * sizeof(Glyph));
> - term.alt [row] = malloc(term.col * sizeof(Glyph));
> + term.line[row] = xmalloc(term.col * sizeof(Glyph));
> + term.alt [row] = xmalloc(term.col * sizeof(Glyph));
> term.dirty[row] = 0;
> }
> memset(term.tabs, 0, term.col * sizeof(*term.tabs));
> _AT_@ -1761,16 +1779,16 @@
> }
>
> /* resize to new height */
> - term.line = realloc(term.line, row * sizeof(Line));
> - term.alt = realloc(term.alt, row * sizeof(Line));
> - term.dirty = realloc(term.dirty, row * sizeof(*term.dirty));
> - term.tabs = realloc(term.tabs, col * sizeof(*term.tabs));
> + term.line = xrealloc(term.line, row * sizeof(Line));
> + term.alt = xrealloc(term.alt, row * sizeof(Line));
> + term.dirty = xrealloc(term.dirty, row * sizeof(*term.dirty));
> + term.tabs = xrealloc(term.tabs, col * sizeof(*term.tabs));
>
> /* resize each row to new width, zero-pad if needed */
> for(i = 0; i < minrow; i++) {
> term.dirty[i] = 1;
> - term.line[i] = realloc(term.line[i], col * sizeof(Glyph));
> - term.alt[i] = realloc(term.alt[i], col * sizeof(Glyph));
> + term.line[i] = xrealloc(term.line[i], col * sizeof(Glyph));
> + term.alt[i] = xrealloc(term.alt[i], col * sizeof(Glyph));
> for(x = mincol; x < col; x++) {
> term.line[i][x].state = 0;
> term.alt[i][x].state = 0;
>
>
Received on Thu Sep 13 2012 - 16:37:51 CEST

This archive was generated by hypermail 2.3.0 : Thu Sep 13 2012 - 16:48:06 CEST