My code

[Guest]

Hi,
I have a structre of a database record,
and I want to write a function that makes a
statement for inserting it into a database.
I have written make_statement_string() function
for this, and I have posted my code here.

But I'm not sure if this code is very good or
efficient. What are your opinion? I try to code
using the KISS principle, but its kinda hard
when you use strings, since you have to allocate
memory for them of the right size.

#include <stdio.h>
#include <stdlib.h>

struct fdesc {
char *name;

/* other irrelevant stuff ... */
};

struct record {
size_t count;
struct fdesc *desc;

/* other record data
* ... */
};

/* Returns a comma-separated string of num question marks */
char *question_marks(size_t num)
{
char *buf, *ptr;
size_t i;

if (num == 0)
return NULL;

buf = malloc((num - 1) * 3 + 2);

ptr = buf;
for (i = 0; i < num - 1; i++) {
strcpy(ptr, "?, ");
ptr += 3;
}
*ptr++ = '?';
*ptr = '\0';

return buf;
}

/* Make a comma-separated list of the names in the record r */
char *get_names(struct record r)
{
size_t i, sz;
size_t index;
char *buf, *ptr;

/* calcuate how much memory we need for the string.
* The number 2 is for a comma and a space */
sz = 0;
for (i = 0; i < r.count; i++)
sz += strlen(r.desc.name) + 2;
/* no comma or space after the last name, but
* keep one for the '\0' */
sz--;

buf = malloc(sz);
index = 0;
for (i = 0; i < r.count; i++) {
if (i != 0) {
strcpy(&buf[index], ", ");
index += 2;
}
ptr = r.desc.name;
while (*ptr != '\0')
buf[index++] = *ptr++;
}
buf[index] = '\0';

return buf;
}

char *make_statement_string(struct record r)
{
char *names, *values;
char *statement;

names = get_names(r);
values = question_marks(r.count);

statement = malloc(30 + strlen(names) + strlen(values));
sprintf(statement, "INSERT INTO table (%s) VALUES(%s)", names, values);
free(names);
free(values);

return statement;
}

 

[Bill Pursell]

Hi,
I have a structre of a database record,
and I want to write a function that makes a
statement for inserting it into a database.
I have written make_statement_string() function
for this, and I have posted my code here.

But I'm not sure if this code is very good or
efficient. What are your opinion? I try to code
using the KISS principle, but its kinda hard
when you use strings, since you have to allocate
memory for them of the right size.

I had a few thoughts. First, assuming that you don't want
to publish the interfaces for any functions other than
make_statement_string(), you can reduce a lot of your
mallocs. Since you are freeing 2/3 of the pointers you
allocate, I would consider making the 2 temporaries
static, and grow them as necessary. I made a few
changes, and put a main function in for testing, which
generated my own question, below the code. Note that
you can do alot better by allocating a large buffer and
doubling it whenver you need to increase the size, or
some other allocation scheme.

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

const char *insert_statement = "INSERT INTO table (%s) VALUES(%s)";

struct fdesc {
char *name;
};

struct record {
unsigned count;
struct fdesc *desc;
};

/* Build a comma-separated string of num question marks */
static void
question_marks(unsigned num, char *buf)
{
char *ptr;

buf[0] = '\0';
ptr = buf;

if (num > 0) {
int i;
for (i=1; i < num; i++, ptr+=3)
strcat(ptr, "?, ");
*ptr++ = '?';
*ptr = '\0';
}
}

/* Make a comma-separated list of the names in the record r */
static void
get_names(struct record r, char *buf)
{
size_t i;
size_t index;
char *ptr;

index = 0;
for (i = 0; i < r.count; i++) {
ptr = r.desc.name;
while (*ptr != '\0')
buf[index++] = *ptr++;
strcpy(buf+index, ", ");
index += 2;
}
buf[index-2] = '\0';
}

/*
*calcuate how much memory we need for the string.
* The number 2 is for a comma and a space
*/
static size_t
get_name_size(const struct record *r)
{
int i;
size_t sz;

sz = 0;
for (i = 0; i < r->count; i++)
sz += strlen(r->desc.name) + 2;
/* no comma or space after the last name, but
* keep one for the '\0' */
sz--;

return sz;
}

static size_t
get_value_size(const struct record *r)
{
return (r->count - 1) * 3 + 2;
}

/*
* A simple wrapper around realloc that exits on
* failure. Note, we call realloc here in a
* non-standard way, discarding the original
* pointer when realloc fails. Since we are
* exiting in that case, we don't care.
*/
static void *
Realloc(size_t old_size, void *ptr, size_t new_size)
{
void *ret;
if (new_size > old_size)
ret = realloc(ptr, new_size);
else
ret = ptr;

if (ret == NULL) {
fprintf(stderr, "Out of memory\n");
exit (EXIT_FAILURE);
}
return ret;
}

char *
make_statement_string(struct record r)
{
static char *names;
static char *values;
static size_t name_size;
static size_t value_size;
char *statement;

names = Realloc(name_size, names, get_name_size(&r));
values = Realloc(value_size, values, get_value_size(&r));
name_size = get_name_size(&r);
value_size = get_value_size(&r);

get_names(r, names);
question_marks(r.count, values);

statement = malloc(strlen(insert_statement) +
strlen(names) + strlen(values));
if (statement != NULL)
sprintf(statement, insert_statement, names, values);

return statement;

}

int
main(void)
{
struct fdesc f[2] = {{"foo"},{"bar"}};
struct fdesc g[] = {{"foo"},{"bar"}, {"baz"}, {"quux"}};
struct record records[] = {{2,f}, {4,g}};
printf("%s\n", make_statement_string(records[0]));
printf("%s\n", make_statement_string(records[1]));
return EXIT_SUCCESS;
}



My question is: when compiled with -pedantic, I get the warning:
warning: initializer element is not computable at load time
in reference to the initializer of records.

Can anyone explain that warning? It looks computable to me.

Also, I hope the above is compilable--I noticed that I missed
at least one '/' at the start of a comment when I
cut/pasted...apologies
if there are other errors.

 

[Guest]

I had a few thoughts. First, assuming that you don't want
to publish the interfaces for any functions other than
make_statement_string(), you can reduce a lot of your
mallocs. Since you are freeing 2/3 of the pointers you
allocate, I would consider making the 2 temporaries
static, and grow them as necessary. I made a few
changes, and put a main function in for testing, which
generated my own question, below the code. Note that
you can do alot better by allocating a large buffer and
doubling it whenver you need to increase the size, or
some other allocation scheme.

Thank you for taking your time to help me!
It's greatly appreciated.

Your code looks nice, and I understand the benefit of
using static storage for the strings. But won't that
make the function non-reentrant, or what its called.
I had the impression that you should avoid using static
in functions.

Also, is there any particular reason you used a pointer
to structure as argument in some functions and not in
the others? The reason I didn't use pointers was to
make you didn't have to use the & operator when calling,
since the struct is so small. But just a matter of
style I guess.

 

[Bill Pursell]

Thank you for taking your time to help me!
It's greatly appreciated.

Your code looks nice, and I understand the benefit of
using static storage for the strings. But won't that
make the function non-reentrant, or what its called.
I had the impression that you should avoid using static
in functions.

Yes, they won't be reentrant. But I'm fairly certain the printf
family isn't reentrant (that's probably platform dependent),
so removing the statics won't fix that. There are arguments
against using static variables, but I think that allocating
space on the heap is worse than using static variables.

Also, is there any particular reason you used a pointer
to structure as argument in some functions and not in
the others? The reason I didn't use pointers was to
make you didn't have to use the & operator when calling,
since the struct is so small. But just a matter of
style I guess.

Stylistically, I prefer passing pointers. There is a
small performance gain with small structures, and
the benefit increases with the size of the structure,
since less data is put on the calling stack. I don't
know how substantial the benefit is. Mostly I do
it because I'm not comfortable returning structures,
and I'm just in the habit of using '->' instead of '.'.

 

[Guest]

Yes, they won't be reentrant. But I'm fairly certain the printf
family isn't reentrant (that's probably platform dependent),
so removing the statics won't fix that. There are arguments
against using static variables, but I think that allocating
space on the heap is worse than using static variables.

Allocating on the heap, that is using malloc, right? Why is that
wore than statics, is it because of speed?

 

[Bill Pursell]

Allocating on the heap, that is using malloc, right? Why is that
wore than statics, is it because of speed?

There is a performance issue, but usually I like to avoid
using malloc because it's often difficult to deal with the
error condition. I try to always arrange my code so that
it does all it's allocation at startup, and it can abort if
memory isn't available. Calling malloc in the middle
of a long-running process where it may be inconvenient
to abort is usually more difficult to deal with.

I use the phrase "allocate on the heap" to refer to the
whole malloc/calloc/realloc family. It may be
technically inaccurate, but I like to keep a naive
model of memory in my head: local automatic variables
are on the stack, everything else is on the heap. So far,
it's worked for me.

 

[Barry Schwarz]

Allocating on the heap, that is using malloc, right? Why is that
wore than statics, is it because of speed?

C has dynamic allocation, courtesy of malloc and friends. It does not
have heaps or stacks. Most references to stacks and heaps are
actually referring to storage duration (static, automatic, and
allocated).


Remove del for email

 

[Keith Thompson]

I use the phrase "allocate on the heap" to refer to the
whole malloc/calloc/realloc family. It may be
technically inaccurate, but I like to keep a naive
model of memory in my head: local automatic variables
are on the stack, everything else is on the heap. So far,
it's worked for me.

C has three storage durations: automatic, static, and allocated.
Objects with automatic storage durations are typically allocated on a
"stack". Objects with "allocated" storage duration (i.e., allocated
by malloc, calloc, or realloc) are typically allocated on a "heap".
Objects with static storage duration typically are allocated at
program startup, and are not on either the stack or the heap.

Of course, the C standard doesn't refer to "stacks" or "heaps".
Automatic objects are allocated in a logically stack-like manner
(first-in last-out), but an implementation could legally allocate the
local objects for each function call by calling malloc() or
equivalent; in fact, I've heard of implementations that do this.