Skip to content

Commit

Permalink
llist: no longer uses malloc
Browse files Browse the repository at this point in the history 
The 'list element' struct now has to be within the data that is being
added to the list. Removes 16.6% (tiny) mallocs from a simple HTTP
transfer. (96 => 80)

Also removed return codes since the llist functions can't fail now.

Test 1300 updated accordingly.

Closes #1435
  • Loading branch information
@bagder
bagder committed Apr 22, 2017
1 parent cbb59ed commit cbae73e
Show file tree
Hide file tree
Showing 13 changed files with 170 additions and 208 deletions.
8 changes: 3 additions & 5 deletions lib/conncache.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,13 +72,11 @@ static void bundle_destroy(struct connectbundle *cb_ptr)

/* Add a connection to a bundle */
static CURLcode bundle_add_conn(struct connectbundle *cb_ptr,
struct connectdata *conn)
struct connectdata *conn)
{
if(!Curl_llist_insert_next(&cb_ptr->conn_list, cb_ptr->conn_list.tail, conn))
return CURLE_OUT_OF_MEMORY;

Curl_llist_insert_next(&cb_ptr->conn_list, cb_ptr->conn_list.tail, conn,
&conn->bundle_node);
conn->bundle = cb_ptr;

cb_ptr->num_connections++;
return CURLE_OK;
}
Expand Down
14 changes: 5 additions & 9 deletions lib/fileinfo.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* | (__| |_| | _ <| |___
* \___|\___/|_| \_\_____|
*
* Copyright (C) 2010 - 2015, Daniel Stenberg, <daniel@haxx.se>, et al.
* Copyright (C) 2010 - 2017, Daniel Stenberg, <daniel@haxx.se>, et al.
*
* This software is licensed as described in the file COPYING, which
* you should have received as part of this distribution. The terms
Expand All @@ -28,23 +28,19 @@
/* The last #include file should be: */
#include "memdebug.h"

struct curl_fileinfo *Curl_fileinfo_alloc(void)
struct fileinfo *Curl_fileinfo_alloc(void)
{
struct curl_fileinfo *tmp = malloc(sizeof(struct curl_fileinfo));
if(!tmp)
return NULL;
memset(tmp, 0, sizeof(struct curl_fileinfo));
return tmp;
return calloc(1, sizeof(struct fileinfo));
}

void Curl_fileinfo_dtor(void *user, void *element)
{
struct curl_fileinfo *finfo = element;
struct fileinfo *finfo = element;
(void) user;
if(!finfo)
return;

Curl_safefree(finfo->b_data);
Curl_safefree(finfo->info.b_data);

free(finfo);
}
12 changes: 8 additions & 4 deletions lib/fileinfo.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
* | (__| |_| | _ <| |___
* \___|\___/|_| \_\_____|
*
* Copyright (C) 2010, Daniel Stenberg, <daniel@haxx.se>, et al.
* Copyright (C) 2010, 2017, Daniel Stenberg, <daniel@haxx.se>, et al.
*
* This software is licensed as described in the file COPYING, which
* you should have received as part of this distribution. The terms
Expand All @@ -23,11 +23,15 @@
***************************************************************************/

#include <curl/curl.h>
#include "llist.h"

struct curl_fileinfo *Curl_fileinfo_alloc(void);
struct fileinfo {
struct curl_fileinfo info;
struct curl_llist_element list;
};

void Curl_fileinfo_dtor(void *, void *);
struct fileinfo *Curl_fileinfo_alloc(void);

struct curl_fileinfo *Curl_fileinfo_dup(const struct curl_fileinfo *src);
void Curl_fileinfo_dtor(void *, void *);

#endif /* HEADER_CURL_FILEINFO_H */
49 changes: 24 additions & 25 deletions lib/ftplistparser.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ struct ftp_parselist_data {
} state;

CURLcode error;
struct curl_fileinfo *file_data;
struct fileinfo *file_data;
unsigned int item_length;
size_t item_offset;
struct {
Expand Down Expand Up @@ -275,14 +275,15 @@ static void PL_ERROR(struct connectdata *conn, CURLcode err)
}

static CURLcode ftp_pl_insert_finfo(struct connectdata *conn,
struct curl_fileinfo *finfo)
struct fileinfo *infop)
{
curl_fnmatch_callback compare;
struct WildcardData *wc = &conn->data->wildcard;
struct ftp_wc_tmpdata *tmpdata = wc->tmp;
struct curl_llist *llist = &wc->filelist;
struct ftp_parselist_data *parser = tmpdata->parser;
bool add = TRUE;
struct curl_fileinfo *finfo = &infop->info;

/* move finfo pointers to b_data */
char *str = finfo->b_data;
Expand Down Expand Up @@ -316,11 +317,7 @@ static CURLcode ftp_pl_insert_finfo(struct connectdata *conn,
}

if(add) {
if(!Curl_llist_insert_next(llist, llist->tail, finfo)) {
Curl_fileinfo_dtor(NULL, finfo);
tmpdata->parser->file_data = NULL;
return CURLE_OUT_OF_MEMORY;
}
Curl_llist_insert_next(llist, llist->tail, finfo, &infop->list);
}
else {
Curl_fileinfo_dtor(NULL, finfo);
Expand All @@ -337,6 +334,7 @@ size_t Curl_ftp_parselist(char *buffer, size_t size, size_t nmemb,
struct connectdata *conn = (struct connectdata *)connptr;
struct ftp_wc_tmpdata *tmpdata = conn->data->wildcard.tmp;
struct ftp_parselist_data *parser = tmpdata->parser;
struct fileinfo *infop;
struct curl_fileinfo *finfo;
unsigned long i = 0;
CURLcode result;
Expand Down Expand Up @@ -366,17 +364,18 @@ size_t Curl_ftp_parselist(char *buffer, size_t size, size_t nmemb,
parser->error = CURLE_OUT_OF_MEMORY;
return bufflen;
}
parser->file_data->b_data = malloc(FTP_BUFFER_ALLOCSIZE);
if(!parser->file_data->b_data) {
parser->file_data->info.b_data = malloc(FTP_BUFFER_ALLOCSIZE);
if(!parser->file_data->info.b_data) {
PL_ERROR(conn, CURLE_OUT_OF_MEMORY);
return bufflen;
}
parser->file_data->b_size = FTP_BUFFER_ALLOCSIZE;
parser->file_data->info.b_size = FTP_BUFFER_ALLOCSIZE;
parser->item_offset = 0;
parser->item_length = 0;
}

finfo = parser->file_data;
infop = parser->file_data;
finfo = &infop->info;
finfo->b_data[finfo->b_used++] = c;

if(finfo->b_used >= finfo->b_size - 1) {
Expand Down Expand Up @@ -498,8 +497,8 @@ size_t Curl_ftp_parselist(char *buffer, size_t size, size_t nmemb,
PL_ERROR(conn, CURLE_FTP_BAD_FILE_LIST);
return bufflen;
}
parser->file_data->flags |= CURLFINFOFLAG_KNOWN_PERM;
parser->file_data->perm = perm;
parser->file_data->info.flags |= CURLFINFOFLAG_KNOWN_PERM;
parser->file_data->info.perm = perm;
parser->offsets.perm = parser->item_offset;

parser->item_length = 0;
Expand Down Expand Up @@ -530,8 +529,8 @@ size_t Curl_ftp_parselist(char *buffer, size_t size, size_t nmemb,
finfo->b_data[parser->item_offset + parser->item_length - 1] = 0;
hlinks = strtol(finfo->b_data + parser->item_offset, &p, 10);
if(p[0] == '\0' && hlinks != LONG_MAX && hlinks != LONG_MIN) {
parser->file_data->flags |= CURLFINFOFLAG_KNOWN_HLINKCOUNT;
parser->file_data->hardlinks = hlinks;
parser->file_data->info.flags |= CURLFINFOFLAG_KNOWN_HLINKCOUNT;
parser->file_data->info.hardlinks = hlinks;
}
parser->item_length = 0;
parser->item_offset = 0;
Expand Down Expand Up @@ -613,8 +612,8 @@ size_t Curl_ftp_parselist(char *buffer, size_t size, size_t nmemb,
fsize = curlx_strtoofft(finfo->b_data+parser->item_offset, &p, 10);
if(p[0] == '\0' && fsize != CURL_OFF_T_MAX &&
fsize != CURL_OFF_T_MIN) {
parser->file_data->flags |= CURLFINFOFLAG_KNOWN_SIZE;
parser->file_data->size = fsize;
parser->file_data->info.flags |= CURLFINFOFLAG_KNOWN_SIZE;
parser->file_data->info.size = fsize;
}
parser->item_length = 0;
parser->item_offset = 0;
Expand Down Expand Up @@ -731,7 +730,7 @@ size_t Curl_ftp_parselist(char *buffer, size_t size, size_t nmemb,
finfo->b_data[parser->item_offset + parser->item_length - 1] = 0;
parser->offsets.filename = parser->item_offset;
parser->state.UNIX.main = PL_UNIX_FILETYPE;
result = ftp_pl_insert_finfo(conn, finfo);
result = ftp_pl_insert_finfo(conn, infop);
if(result) {
PL_ERROR(conn, result);
return bufflen;
Expand All @@ -743,7 +742,7 @@ size_t Curl_ftp_parselist(char *buffer, size_t size, size_t nmemb,
finfo->b_data[parser->item_offset + parser->item_length - 1] = 0;
parser->offsets.filename = parser->item_offset;
parser->state.UNIX.main = PL_UNIX_FILETYPE;
result = ftp_pl_insert_finfo(conn, finfo);
result = ftp_pl_insert_finfo(conn, infop);
if(result) {
PL_ERROR(conn, result);
return bufflen;
Expand Down Expand Up @@ -838,7 +837,7 @@ size_t Curl_ftp_parselist(char *buffer, size_t size, size_t nmemb,
else if(c == '\n') {
finfo->b_data[parser->item_offset + parser->item_length - 1] = 0;
parser->offsets.symlink_target = parser->item_offset;
result = ftp_pl_insert_finfo(conn, finfo);
result = ftp_pl_insert_finfo(conn, infop);
if(result) {
PL_ERROR(conn, result);
return bufflen;
Expand All @@ -850,7 +849,7 @@ size_t Curl_ftp_parselist(char *buffer, size_t size, size_t nmemb,
if(c == '\n') {
finfo->b_data[parser->item_offset + parser->item_length - 1] = 0;
parser->offsets.symlink_target = parser->item_offset;
result = ftp_pl_insert_finfo(conn, finfo);
result = ftp_pl_insert_finfo(conn, infop);
if(result) {
PL_ERROR(conn, result);
return bufflen;
Expand Down Expand Up @@ -953,10 +952,10 @@ size_t Curl_ftp_parselist(char *buffer, size_t size, size_t nmemb,
return bufflen;
}
/* correct file type */
parser->file_data->filetype = CURLFILETYPE_FILE;
parser->file_data->info.filetype = CURLFILETYPE_FILE;
}

parser->file_data->flags |= CURLFINFOFLAG_KNOWN_SIZE;
parser->file_data->info.flags |= CURLFINFOFLAG_KNOWN_SIZE;
parser->item_length = 0;
parser->state.NT.main = PL_WINNT_FILENAME;
parser->state.NT.sub.filename = PL_WINNT_FILENAME_PRESPACE;
Expand All @@ -983,7 +982,7 @@ size_t Curl_ftp_parselist(char *buffer, size_t size, size_t nmemb,
parser->offsets.filename = parser->item_offset;
finfo->b_data[finfo->b_used - 1] = 0;
parser->offsets.filename = parser->item_offset;
result = ftp_pl_insert_finfo(conn, finfo);
result = ftp_pl_insert_finfo(conn, infop);
if(result) {
PL_ERROR(conn, result);
return bufflen;
Expand All @@ -995,7 +994,7 @@ size_t Curl_ftp_parselist(char *buffer, size_t size, size_t nmemb,
case PL_WINNT_FILENAME_WINEOL:
if(c == '\n') {
parser->offsets.filename = parser->item_offset;
result = ftp_pl_insert_finfo(conn, finfo);
result = ftp_pl_insert_finfo(conn, infop);
if(result) {
PL_ERROR(conn, result);
return bufflen;
Expand Down
14 changes: 3 additions & 11 deletions lib/hash.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,17 +124,9 @@ Curl_hash_add(struct curl_hash *h, void *key, size_t key_len, void *p)

he = mk_hash_element(key, key_len, p);
if(he) {
if(Curl_llist_insert_next(l, l->tail, he)) {
++h->size;
return p; /* return the new entry */
}
/*
* Couldn't insert it, destroy the 'he' element and the key again. We
* don't call hash_element_dtor() since that would also call the
* "destructor" for the actual data 'p'. When we fail, we shall not touch
* that data.
*/
free(he);
Curl_llist_insert_next(l, l->tail, he, &he->list);
++h->size;
return p; /* return the new entry */
}

return NULL; /* failure */
Expand Down
1 change: 1 addition & 0 deletions lib/hash.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ struct curl_hash {
};

struct curl_hash_element {
struct curl_llist_element list;
void *ptr;
size_t key_len;
char key[1]; /* allocated memory following the struct */
Expand Down
34 changes: 15 additions & 19 deletions lib/llist.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,18 +49,17 @@ Curl_llist_init(struct curl_llist *l, curl_llist_dtor dtor)
* entry is NULL and the list already has elements, the new one will be
* inserted first in the list.
*
* The 'ne' argument should be a pointer into the object to store.
*
* Returns: 1 on success and 0 on failure.

This comment has been minimized.

Sign in to view

Copy link
@kevinji

kevinji Apr 23, 2017

@bagder Looks like this comment didn't get updated.

This comment has been minimized.

Sign in to view

Copy link
@bagder

bagder Apr 23, 2017

Author Member

Thanks, fixed in d87bd46
*
* @unittest: 1300
*/
int
void
Curl_llist_insert_next(struct curl_llist *list, struct curl_llist_element *e,
const void *p)
const void *p,
struct curl_llist_element *ne)
{
struct curl_llist_element *ne = malloc(sizeof(struct curl_llist_element));
if(!ne)
return 0;

ne->ptr = (void *) p;
if(list->size == 0) {
list->head = ne;
Expand All @@ -87,19 +86,18 @@ Curl_llist_insert_next(struct curl_llist *list, struct curl_llist_element *e,
}

++list->size;

return 1;
}

/*
* @unittest: 1300
*/
int
void
Curl_llist_remove(struct curl_llist *list, struct curl_llist_element *e,
void *user)
{
void *ptr;
if(e == NULL || list->size == 0)
return 1;
return;

if(e == list->head) {
list->head = e->next;
Expand All @@ -117,16 +115,16 @@ Curl_llist_remove(struct curl_llist *list, struct curl_llist_element *e,
e->next->prev = e->prev;
}

list->dtor(user, e->ptr);
ptr = e->ptr;

e->ptr = NULL;
e->prev = NULL;
e->next = NULL;

free(e);
--list->size;

return 1;
/* call the dtor() last for when it actually frees the 'e' memory itself */
list->dtor(user, ptr);
}

void
Expand All @@ -147,13 +145,13 @@ Curl_llist_count(struct curl_llist *list)
/*
* @unittest: 1300
*/
int Curl_llist_move(struct curl_llist *list, struct curl_llist_element *e,
struct curl_llist *to_list,
struct curl_llist_element *to_e)
void Curl_llist_move(struct curl_llist *list, struct curl_llist_element *e,
struct curl_llist *to_list,
struct curl_llist_element *to_e)
{
/* Remove element from list */
if(e == NULL || list->size == 0)
return 0;
return;

if(e == list->head) {
list->head = e->next;
Expand Down Expand Up @@ -193,6 +191,4 @@ int Curl_llist_move(struct curl_llist *list, struct curl_llist_element *e,
}

++to_list->size;

return 1;
}
Loading

0 comments on commit cbae73e

Please sign in to comment.