Skip to content

Commit 5851901

Browse files
author
Vicent Marti
committed
Fix internal memory management on the library
String mememory is now managed in a much more sane manner. Fixes include: - git_person email and name is no longer limited to 64 characters - git_tree_entry filename is no longer limited to 255 characters - raw objects are properly opened & closed the minimum amount of times required for parsing - unit tests no longer leak - removed 5 other misc memory leaks as reported by Valgrind - tree writeback no longer segfaults on rare ocassions The git_person struct is no longer public. It is now managed by the library, and getter methods are in place to access its internal attributes. Signed-off-by: Vicent Marti <tanoku@gmail.com>
1 parent 2d16373 commit 5851901

File tree

17 files changed

+395
-280
lines changed

17 files changed

+395
-280
lines changed

‎src/commit.c‎

Lines changed: 68 additions & 164 deletions
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,14 @@
2323
* Boston, MA 02110-1301, USA.
2424
*/
2525

26+
#include "git/common.h"
27+
#include "git/odb.h"
28+
#include "git/repository.h"
29+
2630
#include "common.h"
2731
#include "commit.h"
2832
#include "revwalk.h"
29-
#include "git/odb.h"
30-
#include "git/repository.h"
33+
#include "person.h"
3134

3235
#define COMMIT_BASIC_PARSE 0x0
3336
#define COMMIT_FULL_PARSE 0x1
@@ -56,8 +59,9 @@ void git_commit__free(git_commit *commit)
5659
{
5760
clear_parents(commit);
5861

59-
free(commit->author);
60-
free(commit->committer);
62+
git_person__free(commit->author);
63+
git_person__free(commit->committer);
64+
6165
free(commit->message);
6266
free(commit->message_short);
6367
free(commit);
@@ -73,146 +77,12 @@ const git_oid *git_commit_id(git_commit *c)
7377
return git_object_id((git_object *)c);
7478
}
7579

76-
int git_commit__parse(git_commit *commit)
77-
{
78-
const int close_db_object = 1;
79-
int error = 0;
80-
81-
if ((error = git_object__source_open((git_object *)commit)) < 0)
82-
return error;
83-
84-
error = git_commit__parse_buffer(commit,
85-
commit->object.source.raw.data, commit->object.source.raw.len, COMMIT_BASIC_PARSE);
86-
87-
if (close_db_object)
88-
git_object__source_close((git_object *)commit);
89-
90-
return error;
91-
}
92-
93-
int git_commit__parse_full(git_commit *commit)
94-
{
95-
int error;
96-
97-
if (commit->full_parse)
98-
return 0;
99-
100-
if (git_object__source_open((git_object *)commit) < 0)
101-
return GIT_ERROR;
102-
103-
error = git_commit__parse_buffer(commit,
104-
commit->object.source.raw.data, commit->object.source.raw.len, COMMIT_FULL_PARSE);
105-
106-
git_object__source_close((git_object *)commit);
107-
108-
commit->full_parse = 1;
109-
return error;
110-
}
11180

11281
git_commit *git_commit_lookup(git_repository *repo, const git_oid *id)
11382
{
11483
return (git_commit *)git_repository_lookup(repo, id, GIT_OBJ_COMMIT);
11584
}
11685

117-
int git__parse_person(git_person *person, char **buffer_out,
118-
const char *buffer_end, const char *header)
119-
{
120-
const size_t header_len = strlen(header);
121-
122-
int i;
123-
char *buffer = *buffer_out;
124-
char *line_end, *name, *email;
125-
126-
line_end = memchr(buffer, '\n', buffer_end - buffer);
127-
if (!line_end)
128-
return GIT_EOBJCORRUPTED;
129-
130-
if (buffer + (header_len + 1) > line_end)
131-
return GIT_EOBJCORRUPTED;
132-
133-
if (memcmp(buffer, header, header_len) != 0)
134-
return GIT_EOBJCORRUPTED;
135-
136-
buffer += header_len;
137-
138-
139-
/* Parse name field */
140-
for (i = 0, name = person->name;
141-
i < 64 && buffer < line_end && *buffer != '<';
142-
++i)
143-
*name++ = *buffer++;
144-
145-
*(name - 1) = 0;
146-
147-
while (buffer < line_end && *buffer != '<')
148-
buffer++;
149-
150-
if (++buffer >= line_end)
151-
return GIT_EOBJCORRUPTED;
152-
153-
/* Parse email field */
154-
for (i = 0, email = person->email;
155-
i < 64 && buffer < line_end && *buffer != '>';
156-
++i)
157-
*email++ = *buffer++;
158-
159-
*email = 0;
160-
161-
while (buffer < line_end && *buffer != '>')
162-
buffer++;
163-
164-
if (++buffer >= line_end)
165-
return GIT_EOBJCORRUPTED;
166-
167-
person->time = strtol(buffer, &buffer, 10);
168-
169-
if (person->time == 0)
170-
return GIT_EOBJCORRUPTED;
171-
172-
*buffer_out = (line_end + 1);
173-
return 0;
174-
}
175-
176-
int git__write_person(git_odb_source *src, const char *header, const git_person *person)
177-
{
178-
return git__source_printf(src, "%s %s <%s> %u\n", header, person->name, person->email, person->time);
179-
}
180-
181-
int git__parse_oid(git_oid *oid, char **buffer_out,
182-
const char *buffer_end, const char *header)
183-
{
184-
const size_t sha_len = GIT_OID_HEXSZ;
185-
const size_t header_len = strlen(header);
186-
187-
char *buffer = *buffer_out;
188-
189-
if (buffer + (header_len + sha_len + 1) > buffer_end)
190-
return GIT_EOBJCORRUPTED;
191-
192-
if (memcmp(buffer, header, header_len) != 0)
193-
return GIT_EOBJCORRUPTED;
194-
195-
if (buffer[header_len + sha_len] != '\n')
196-
return GIT_EOBJCORRUPTED;
197-
198-
if (git_oid_mkstr(oid, buffer + header_len) < 0)
199-
return GIT_EOBJCORRUPTED;
200-
201-
*buffer_out = buffer + (header_len + sha_len + 1);
202-
203-
return 0;
204-
}
205-
206-
int git__write_oid(git_odb_source *src, const char *header, const git_oid *oid)
207-
{
208-
char hex_oid[41];
209-
210-
git_oid_fmt(hex_oid, oid);
211-
hex_oid[40] = 0;
212-
213-
return git__source_printf(src, "%s %s\n", header, hex_oid);
214-
}
215-
21686
int git_commit__writeback(git_commit *commit, git_odb_source *src)
21787
{
21888
git_commit_parents *parent;
@@ -232,26 +102,25 @@ int git_commit__writeback(git_commit *commit, git_odb_source *src)
232102
if (commit->author == NULL)
233103
return GIT_ERROR;
234104

235-
git__write_person(src, "author", commit->author);
105+
git_person__write(src, "author", commit->author);
236106

237107
if (commit->committer == NULL)
238108
return GIT_ERROR;
239109

240-
git__write_person(src, "committer", commit->committer);
110+
git_person__write(src, "committer", commit->committer);
241111

242112
if (commit->message != NULL)
243113
git__source_printf(src, "\n%s", commit->message);
244114

245115
return GIT_SUCCESS;
246116
}
247117

248-
int git_commit__parse_buffer(git_commit *commit, void *data, size_t len, unsigned int parse_flags)
118+
int commit_parse_buffer(git_commit *commit, void *data, size_t len, unsigned int parse_flags)
249119
{
250120
char *buffer = (char *)data;
251121
const char *buffer_end = (char *)data + len;
252122

253123
git_oid oid;
254-
git_person person;
255124

256125
if (git__parse_oid(&oid, &buffer, buffer_end, "tree ") < 0)
257126
return GIT_EOBJCORRUPTED;
@@ -279,29 +148,31 @@ int git_commit__parse_buffer(git_commit *commit, void *data, size_t len, unsigne
279148
commit->parents = node;
280149
}
281150

282-
if (git__parse_person(&person, &buffer, buffer_end, "author ") < 0)
283-
return GIT_EOBJCORRUPTED;
284151

285152
if (parse_flags & COMMIT_FULL_PARSE) {
286153
if (commit->author)
287-
free(commit->author);
154+
git_person__free(commit->author);
288155

289156
commit->author = git__malloc(sizeof(git_person));
290-
memcpy(commit->author, &person, sizeof(git_person));
291-
}
157+
if (git_person__parse(commit->author, &buffer, buffer_end, "author ") < 0)
158+
return GIT_EOBJCORRUPTED;
292159

293-
if (git__parse_person(&person, &buffer, buffer_end, "committer ") < 0)
294-
return GIT_EOBJCORRUPTED;
160+
} else {
161+
if ((buffer = memchr(buffer, '\n', buffer_end - buffer)) == NULL)
162+
return GIT_EOBJCORRUPTED;
295163

296-
commit->commit_time = person.time;
164+
buffer++;
165+
}
297166

298-
if (parse_flags & COMMIT_FULL_PARSE) {
299-
if (commit->committer)
300-
free(commit->committer);
167+
/* Always parse the committer; we need the commit time */
168+
if (commit->committer)
169+
git_person__free(commit->committer);
301170

302-
commit->committer = git__malloc(sizeof(git_person));
303-
memcpy(commit->committer, &person, sizeof(git_person));
304-
}
171+
commit->committer = git__malloc(sizeof(git_person));
172+
if (git_person__parse(commit->committer, &buffer, buffer_end, "committer ") < 0)
173+
return GIT_EOBJCORRUPTED;
174+
175+
commit->commit_time = commit->committer->time;
305176

306177
/* parse commit message */
307178
while (buffer <= buffer_end && *buffer == '\n')
@@ -329,9 +200,38 @@ int git_commit__parse_buffer(git_commit *commit, void *data, size_t len, unsigne
329200
return 0;
330201
}
331202

203+
int git_commit__parse(git_commit *commit)
204+
{
205+
assert(commit && commit->object.source.open);
206+
return commit_parse_buffer(commit,
207+
commit->object.source.raw.data, commit->object.source.raw.len, COMMIT_BASIC_PARSE);
208+
}
209+
210+
int git_commit__parse_full(git_commit *commit)
211+
{
212+
int error;
213+
214+
if (commit->full_parse)
215+
return 0;
216+
217+
if (git_object__source_open((git_object *)commit) < 0)
218+
return GIT_ERROR;
219+
220+
error = commit_parse_buffer(commit,
221+
commit->object.source.raw.data, commit->object.source.raw.len, COMMIT_FULL_PARSE);
222+
223+
git_object__source_close((git_object *)commit);
224+
225+
commit->full_parse = 1;
226+
return error;
227+
}
228+
229+
230+
332231
#define GIT_COMMIT_GETTER(_rvalue, _name) \
333232
const _rvalue git_commit_##_name(git_commit *commit) \
334233
{\
234+
assert(commit); \
335235
if (commit->_name) \
336236
return commit->_name; \
337237
git_commit__parse_full(commit); \
@@ -346,6 +246,8 @@ GIT_COMMIT_GETTER(char *, message_short)
346246

347247
time_t git_commit_time(git_commit *commit)
348248
{
249+
assert(commit);
250+
349251
if (commit->commit_time)
350252
return commit->commit_time;
351253

@@ -355,26 +257,27 @@ time_t git_commit_time(git_commit *commit)
355257

356258
void git_commit_set_tree(git_commit *commit, git_tree *tree)
357259
{
260+
assert(commit && tree);
358261
commit->object.modified = 1;
359262
commit->tree = tree;
360263
}
361264

362-
void git_commit_set_author(git_commit *commit, const git_person *author)
265+
void git_commit_set_author(git_commit *commit, const char *name, const char *email, time_t time)
363266
{
267+
assert(commit && name && email);
364268
commit->object.modified = 1;
365-
if (commit->author == NULL)
366-
commit->author = git__malloc(sizeof(git_person));
367269

368-
memcpy(commit->author, author, sizeof(git_person));
270+
git_person__free(commit->author);
271+
commit->author = git_person__new(name, email, time);
369272
}
370273

371-
void git_commit_set_committer(git_commit *commit, const git_person *committer)
274+
void git_commit_set_committer(git_commit *commit, const char *name, const char *email, time_t time)
372275
{
276+
assert(commit && name && email);
373277
commit->object.modified = 1;
374-
if (commit->committer == NULL)
375-
commit->committer = git__malloc(sizeof(git_person));
376278

377-
memcpy(commit->committer, committer, sizeof(git_person));
279+
git_person__free(commit->committer);
280+
commit->committer = git_person__new(name, email, time);
378281
}
379282

380283
void git_commit_set_message(git_commit *commit, const char *message)
@@ -409,3 +312,4 @@ void git_commit_add_parent(git_commit *commit, git_commit *new_parent)
409312
commit->parents = node;
410313
}
411314

315+

‎src/commit.h‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -31,14 +31,7 @@ struct git_commit {
3131
void git_commit__free(git_commit *c);
3232
int git_commit__parse(git_commit *commit);
3333
int git_commit__parse_full(git_commit *commit);
34-
int git_commit__parse_buffer(git_commit *commit, void *data, size_t len, unsigned int parse_flags);
3534

3635
int git_commit__writeback(git_commit *commit, git_odb_source *src);
3736

38-
int git__parse_oid(git_oid *oid, char **buffer_out, const char *buffer_end, const char *header);
39-
int git__parse_person(git_person *person, char **buffer_out, const char *buffer_end, const char *header);
40-
41-
int git__write_oid(git_odb_source *src, const char *header, const git_oid *oid);
42-
int git__write_person(git_odb_source *src, const char *header, const git_person *person);
43-
4437
#endif

‎src/git/commit.h‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -108,16 +108,20 @@ GIT_EXTERN(void) git_commit_set_message(git_commit *commit, const char *message)
108108
/**
109109
* Set the committer of a commit
110110
* @param commit the commit object
111-
* @param committer the new committer
111+
* @param name name of the new committer
112+
* @param email email of the new committer
113+
* @param time time when the committer committed the commit
112114
*/
113-
GIT_EXTERN(void) git_commit_set_committer(git_commit *commit, const git_person *committer);
115+
GIT_EXTERN(void) git_commit_set_committer(git_commit *commit, const char *name, const char *email, time_t time);
114116

115117
/**
116118
* Set the author of a commit
117119
* @param commit the commit object
118-
* @param author the new author
120+
* @param name name of the new author
121+
* @param email email of the new author
122+
* @param time time when the author created the commit
119123
*/
120-
GIT_EXTERN(void) git_commit_set_author(git_commit *commit, const git_person *author);
124+
GIT_EXTERN(void) git_commit_set_author(git_commit *commit, const char *name, const char *email, time_t time);
121125

122126
/**
123127
* Set the tree which is pointed to by a commit

0 commit comments

Comments
 (0)