Prefer binary realloc to self-managed buffers

Use erlang's allocator and simplify the code by writing into the
buffer we're going to give back anyway.

Also introduce a function to NUL-terminate iolist buffers, which can
avoid memory copying.
This commit is contained in:
Carlos Martín Nieto
2013-05-02 13:09:51 +02:00
parent ef7d2d5490
commit 616a358dc7
6 changed files with 33 additions and 89 deletions

View File

@@ -40,7 +40,6 @@ geef_commit_tree(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[])
term_obj = enif_make_resource(env, obj); term_obj = enif_make_resource(env, obj);
enif_release_resource(obj); enif_release_resource(obj);
return enif_make_tuple3(env, atoms.ok, atoms.tree, term_obj); return enif_make_tuple3(env, atoms.ok, atoms.tree, term_obj);
} }

View File

@@ -123,6 +123,16 @@ geef_oom(ErlNifEnv *env)
return enif_make_tuple2(env, atoms.error, atoms.enomem); return enif_make_tuple2(env, atoms.error, atoms.enomem);
} }
int geef_terminate_binary(ErlNifBinary *bin)
{
if (!enif_realloc_binary(bin, bin->size + 1))
return 0;
bin->data[bin->size - 1] = '\0';
return 1;
}
static ErlNifFunc geef_funcs[] = static ErlNifFunc geef_funcs[] =
{ {
{"repository_init", 2, geef_repository_init}, {"repository_init", 2, geef_repository_init},

View File

@@ -29,4 +29,7 @@ typedef struct {
extern geef_atoms atoms; extern geef_atoms atoms;
/** NUL-terminate a binary */
int geef_terminate_binary(ErlNifBinary *bin);
#endif #endif

View File

@@ -49,7 +49,6 @@ geef_reference_lookup(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[])
git_reference *ref; git_reference *ref;
ErlNifBinary bin; ErlNifBinary bin;
ERL_NIF_TERM term_ref; ERL_NIF_TERM term_ref;
char *name;
int error; int error;
if (!enif_get_resource(env, argv[0], geef_repository_type, (void **) &repo)) if (!enif_get_resource(env, argv[0], geef_repository_type, (void **) &repo))
@@ -58,15 +57,10 @@ geef_reference_lookup(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[])
if (!enif_inspect_iolist_as_binary(env, argv[1], &bin)) if (!enif_inspect_iolist_as_binary(env, argv[1], &bin))
return enif_make_badarg(env); return enif_make_badarg(env);
name = malloc(bin.size + 1); if (!geef_terminate_binary(&bin))
if (!name)
return geef_oom(env); return geef_oom(env);
memcpy(name, bin.data, bin.size); error = git_reference_lookup(&ref, repo->repo, (char *)bin.data);
name[bin.size] = '\0';
error = git_reference_lookup(&ref, repo->repo, name);
free(name);
if (error < 0) if (error < 0)
return geef_error(env); return geef_error(env);
@@ -128,7 +122,6 @@ static int append_to_list(const char *name, void *payload)
ERL_NIF_TERM ERL_NIF_TERM
geef_reference_glob(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[]) geef_reference_glob(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[])
{ {
char *glob;
int error; int error;
geef_repository *repo; geef_repository *repo;
ErlNifBinary bin; ErlNifBinary bin;
@@ -140,18 +133,13 @@ geef_reference_glob(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[])
if (!enif_inspect_iolist_as_binary(env, argv[1], &bin)) if (!enif_inspect_iolist_as_binary(env, argv[1], &bin))
return enif_make_badarg(env); return enif_make_badarg(env);
glob = malloc(bin.size + 1); if (!geef_terminate_binary(&bin))
if (!glob)
return geef_oom(env); return geef_oom(env);
memcpy(glob, bin.data, bin.size);
glob[bin.size] = '\0';
data.env = env; data.env = env;
data.list = enif_make_list(env, 0); data.list = enif_make_list(env, 0);
error = git_reference_foreach_glob(repo->repo, glob, GIT_REF_LISTALL, append_to_list, &data); error = git_reference_foreach_glob(repo->repo, (char *) bin.data, GIT_REF_LISTALL, append_to_list, &data);
free(glob);
if (error < 0) if (error < 0)
return geef_error(env); return geef_error(env);
@@ -195,7 +183,6 @@ geef_reference_to_id(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[])
{ {
geef_repository *repo; geef_repository *repo;
ErlNifBinary bin; ErlNifBinary bin;
char *name;
git_oid id; git_oid id;
if (!enif_get_resource(env, argv[0], geef_repository_type, (void **) &repo)) if (!enif_get_resource(env, argv[0], geef_repository_type, (void **) &repo))
@@ -204,14 +191,10 @@ geef_reference_to_id(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[])
if (!enif_inspect_iolist_as_binary(env, argv[1], &bin)) if (!enif_inspect_iolist_as_binary(env, argv[1], &bin))
return enif_make_badarg(env); return enif_make_badarg(env);
name = malloc(bin.size + 1); if (!geef_terminate_binary(&bin))
if (!name)
return geef_oom(env); return geef_oom(env);
memcpy(name, bin.data, bin.size); if (git_reference_name_to_id(&id, repo->repo, (char *)bin.data) < 0)
name[bin.size] = '\0';
if (git_reference_name_to_id(&id, repo->repo, name) < 0)
return geef_error(env); return geef_error(env);
if (geef_oid_bin(&bin, &id) < 0) if (geef_oid_bin(&bin, &id) < 0)

View File

@@ -20,8 +20,7 @@ void geef_odb_free(ErlNifEnv *env, void *cd)
ERL_NIF_TERM ERL_NIF_TERM
geef_repository_init(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[]) geef_repository_init(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[])
{ {
char *path; int bare;
int error, bare;
git_repository *repo; git_repository *repo;
geef_repository *res_repo; geef_repository *res_repo;
ErlNifBinary bin; ErlNifBinary bin;
@@ -30,19 +29,12 @@ geef_repository_init(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[])
if (!enif_inspect_iolist_as_binary(env, argv[0], &bin)) if (!enif_inspect_iolist_as_binary(env, argv[0], &bin))
return enif_make_badarg(env); return enif_make_badarg(env);
path = malloc(bin.size + 1); if (!geef_terminate_binary(&bin))
if (!path)
return geef_oom(env); return geef_oom(env);
memcpy(path, bin.data, bin.size);
path[bin.size] = '\0';
bare = !enif_compare(argv[1], atoms.true); bare = !enif_compare(argv[1], atoms.true);
error = git_repository_init(&repo, path, bare); if (git_repository_init(&repo, (char *) bin.data, bare) < 0)
free(path);
if (error < 0)
return geef_error(env); return geef_error(env);
res_repo = enif_alloc_resource(geef_repository_type, sizeof(geef_repository)); res_repo = enif_alloc_resource(geef_repository_type, sizeof(geef_repository));
@@ -56,8 +48,6 @@ geef_repository_init(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[])
ERL_NIF_TERM ERL_NIF_TERM
geef_repository_open(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[]) geef_repository_open(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[])
{ {
char *path;
int error;
git_repository *repo; git_repository *repo;
geef_repository *res_repo; geef_repository *res_repo;
ErlNifBinary bin; ErlNifBinary bin;
@@ -66,17 +56,10 @@ geef_repository_open(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[])
if (!enif_inspect_iolist_as_binary(env, argv[0], &bin)) if (!enif_inspect_iolist_as_binary(env, argv[0], &bin))
return enif_make_badarg(env); return enif_make_badarg(env);
path = malloc(bin.size + 1); if (!geef_terminate_binary(&bin))
if (!path)
return geef_oom(env); return geef_oom(env);
memcpy(path, bin.data, bin.size); if (git_repository_open(&repo, (char *) bin.data) < 0)
path[bin.size] = '\0';
error = git_repository_open(&repo, path);
free(path);
if (error < 0)
return geef_error(env); return geef_error(env);
res_repo = enif_alloc_resource(geef_repository_type, sizeof(geef_repository)); res_repo = enif_alloc_resource(geef_repository_type, sizeof(geef_repository));
@@ -90,57 +73,31 @@ geef_repository_open(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[])
ERL_NIF_TERM ERL_NIF_TERM
geef_repository_discover(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[]) geef_repository_discover(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[])
{ {
char *path, *buffer; ErlNifBinary bin, path;
size_t buffer_len;
ErlNifBinary bin, res_bin;
int error; int error;
if (!enif_inspect_iolist_as_binary(env, argv[0], &bin)) if (!enif_inspect_iolist_as_binary(env, argv[0], &bin))
return enif_make_badarg(env); return enif_make_badarg(env);
path = malloc(bin.size + 1); if (!geef_terminate_binary(&bin))
if (!path)
return geef_oom(env); return geef_oom(env);
memcpy(path, bin.data, bin.size); if (!enif_alloc_binary(256, &path))
path[bin.size] = '\0';
buffer_len = 256;
buffer = malloc(buffer_len);
if (!buffer)
return geef_oom(env); return geef_oom(env);
while ((error = git_repository_discover(buffer, buffer_len, path, 0, NULL)) < 0 && while ((error = git_repository_discover((char *) path.data, path.size,(char *) bin.data, 0, NULL)) < 0 &&
giterr_last()->klass == GITERR_REPOSITORY) { giterr_last()->klass == GITERR_REPOSITORY) {
char *tmp; if (!enif_realloc_binary(&path, path.size * 2))
buffer_len *= 2;
tmp = realloc(buffer, buffer_len);
if (!tmp) {
free(path);
free(buffer);
return geef_oom(env); return geef_oom(env);
} }
buffer = tmp;
}
free(path);
if (error < 0) { if (error < 0) {
free(buffer);
return geef_error(env); return geef_error(env);
} }
buffer_len = strlen(buffer); enif_realloc_binary(&path, strlen((char *) path.data));
if (!enif_alloc_binary(buffer_len, &res_bin)) {
free(buffer);
return geef_oom(env);
}
memcpy(res_bin.data, buffer, buffer_len); return enif_make_tuple2(env, atoms.ok, enif_make_binary(env, &path));
free(buffer);
return enif_make_tuple2(env, atoms.ok, enif_make_binary(env, &res_bin));
} }
ERL_NIF_TERM ERL_NIF_TERM

View File

@@ -40,7 +40,6 @@ static ERL_NIF_TERM tree_entry_to_term(ErlNifEnv *env, const git_tree_entry *ent
ERL_NIF_TERM ERL_NIF_TERM
geef_tree_bypath(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[]) geef_tree_bypath(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[])
{ {
char *path;
int error; int error;
geef_object *obj; geef_object *obj;
ErlNifBinary bin; ErlNifBinary bin;
@@ -52,17 +51,10 @@ geef_tree_bypath(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[])
if (!enif_inspect_iolist_as_binary(env, argv[1], &bin)) if (!enif_inspect_iolist_as_binary(env, argv[1], &bin))
return enif_make_badarg(env); return enif_make_badarg(env);
path = malloc(bin.size + 1); if (!geef_terminate_binary(&bin))
if (!path)
return geef_oom(env); return geef_oom(env);
memcpy(path, bin.data, bin.size); if (git_tree_entry_bypath(&entry, (git_tree *)obj->obj, (char *) bin.data) < 0)
path[bin.size] = '\0';
error = git_tree_entry_bypath(&entry, (git_tree *)obj->obj, path);
free(path);
if (error < 0)
return geef_error(env); return geef_error(env);
return tree_entry_to_term(env, entry); return tree_entry_to_term(env, entry);