Skip to content

Commit bd48bf3

Browse files
pks-tpks-t
committed
hash: introduce source files to break include circles
The hash source files have circular include dependencies right now, which shows by our broken generic hash implementation. The "hash.h" header declares two functions and the `git_hash_ctx` typedef before actually including the hash backend header and can only declare the remaining hash functions after the include due to possibly static function declarations inside of the implementation includes. Let's break this cycle and help maintainability by creating a real implementation file for each of the hash implementations. Instead of relying on the exact include order, we now especially avoid the use of `GIT_INLINE` for function declarations.
1 parent bbf034a commit bd48bf3

14 files changed

+217
-161
lines changed

‎cmake/Modules/SelectHashes.cmake‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,10 @@ ELSEIF(SHA1_BACKEND STREQUAL "OpenSSL")
4040
ELSE()
4141
LIST(APPEND LIBGIT2_PC_REQUIRES "openssl")
4242
ENDIF()
43+
FILE(GLOB SRC_SHA1 hash/hash_openssl.c)
4344
ELSEIF(SHA1_BACKEND STREQUAL "CommonCrypto")
4445
SET(GIT_SHA1_COMMON_CRYPTO 1)
46+
FILE(GLOB SRC_SHA1 hash/hash_common_crypto.c)
4547
ELSEIF(SHA1_BACKEND STREQUAL "mbedTLS")
4648
SET(GIT_SHA1_MBEDTLS 1)
4749
FILE(GLOB SRC_SHA1 hash/hash_mbedtls.c)

‎src/hash.h‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,10 @@
1313

1414
typedef struct git_hash_ctx git_hash_ctx;
1515

16-
int git_hash_ctx_init(git_hash_ctx *ctx);
17-
void git_hash_ctx_cleanup(git_hash_ctx *ctx);
16+
typedef struct {
17+
void *data;
18+
size_t len;
19+
} git_buf_vec;
1820

1921
#if defined(GIT_SHA1_COLLISIONDETECT)
2022
# include "hash/hash_collisiondetect.h"
@@ -30,10 +32,10 @@ void git_hash_ctx_cleanup(git_hash_ctx *ctx);
3032
# include "hash/hash_generic.h"
3133
#endif
3234

33-
typedef struct {
34-
void *data;
35-
size_t len;
36-
} git_buf_vec;
35+
int git_hash_global_init(void);
36+
37+
int git_hash_ctx_init(git_hash_ctx *ctx);
38+
void git_hash_ctx_cleanup(git_hash_ctx *ctx);
3739

3840
int git_hash_init(git_hash_ctx *c);
3941
int git_hash_update(git_hash_ctx *c, const void *data, size_t len);

‎src/hash/hash_collisiondetect.c‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
/*
2+
* Copyright (C) the libgit2 contributors. All rights reserved.
3+
*
4+
* This file is part of libgit2, distributed under the GNU GPL v2 with
5+
* a Linking Exception. For full terms see the included COPYING file.
6+
*/
7+
8+
#include "hash_collisiondetect.h"
9+
10+
int git_hash_global_init(void)
11+
{
12+
return 0;
13+
}
14+
15+
int git_hash_ctx_init(git_hash_ctx *ctx)
16+
{
17+
return git_hash_init(ctx);
18+
}
19+
20+
void git_hash_ctx_cleanup(git_hash_ctx *ctx)
21+
{
22+
GIT_UNUSED(ctx);
23+
}
24+
25+
int git_hash_init(git_hash_ctx *ctx)
26+
{
27+
assert(ctx);
28+
SHA1DCInit(&ctx->c);
29+
return 0;
30+
}
31+
32+
int git_hash_update(git_hash_ctx *ctx, const void *data, size_t len)
33+
{
34+
assert(ctx);
35+
SHA1DCUpdate(&ctx->c, data, len);
36+
return 0;
37+
}
38+
39+
int git_hash_final(git_oid *out, git_hash_ctx *ctx)
40+
{
41+
assert(ctx);
42+
if (SHA1DCFinal(out->id, &ctx->c)) {
43+
git_error_set(GIT_ERROR_SHA1, "SHA1 collision attack detected");
44+
return -1;
45+
}
46+
47+
return 0;
48+
}

‎src/hash/hash_collisiondetect.h‎

Lines changed: 1 addition & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -9,43 +9,11 @@
99
#define INCLUDE_hash_hash_collisiondetect_h__
1010

1111
#include "hash.h"
12+
1213
#include "sha1dc/sha1.h"
1314

1415
struct git_hash_ctx {
1516
SHA1_CTX c;
1617
};
1718

18-
#define git_hash_ctx_init(ctx) git_hash_init(ctx)
19-
#define git_hash_ctx_cleanup(ctx)
20-
21-
GIT_INLINE(int) git_hash_global_init(void)
22-
{
23-
return 0;
24-
}
25-
26-
GIT_INLINE(int) git_hash_init(git_hash_ctx *ctx)
27-
{
28-
assert(ctx);
29-
SHA1DCInit(&ctx->c);
30-
return 0;
31-
}
32-
33-
GIT_INLINE(int) git_hash_update(git_hash_ctx *ctx, const void *data, size_t len)
34-
{
35-
assert(ctx);
36-
SHA1DCUpdate(&ctx->c, data, len);
37-
return 0;
38-
}
39-
40-
GIT_INLINE(int) git_hash_final(git_oid *out, git_hash_ctx *ctx)
41-
{
42-
assert(ctx);
43-
if (SHA1DCFinal(out->id, &ctx->c)) {
44-
git_error_set(GIT_ERROR_SHA1, "SHA1 collision attack detected");
45-
return -1;
46-
}
47-
48-
return 0;
49-
}
50-
5119
#endif

‎src/hash/hash_common_crypto.c‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
/*
2+
* Copyright (C) the libgit2 contributors. All rights reserved.
3+
*
4+
* This file is part of libgit2, distributed under the GNU GPL v2 with
5+
* a Linking Exception. For full terms see the included COPYING file.
6+
*/
7+
8+
#include "hash_common_crypto.h"
9+
10+
#define CC_LONG_MAX ((CC_LONG)-1)
11+
12+
int git_hash_global_init(void)
13+
{
14+
return 0;
15+
}
16+
17+
int git_hash_ctx_init(git_hash_ctx *ctx)
18+
{
19+
return git_hash_init(ctx);
20+
}
21+
22+
void git_hash_ctx_cleanup(git_hash_ctx *ctx)
23+
{
24+
GIT_UNUSED(ctx);
25+
}
26+
27+
int git_hash_init(git_hash_ctx *ctx)
28+
{
29+
assert(ctx);
30+
CC_SHA1_Init(&ctx->c);
31+
return 0;
32+
}
33+
34+
int git_hash_update(git_hash_ctx *ctx, const void *_data, size_t len)
35+
{
36+
const unsigned char *data = _data;
37+
38+
assert(ctx);
39+
40+
while (len > 0) {
41+
CC_LONG chunk = (len > CC_LONG_MAX) ? CC_LONG_MAX : (CC_LONG)len;
42+
43+
CC_SHA1_Update(&ctx->c, data, chunk);
44+
45+
data += chunk;
46+
len -= chunk;
47+
}
48+
49+
return 0;
50+
}
51+
52+
int git_hash_final(git_oid *out, git_hash_ctx *ctx)
53+
{
54+
assert(ctx);
55+
CC_SHA1_Final(out->id, &ctx->c);
56+
return 0;
57+
}

‎src/hash/hash_common_crypto.h‎

Lines changed: 0 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -16,46 +16,4 @@ struct git_hash_ctx {
1616
CC_SHA1_CTX c;
1717
};
1818

19-
#define CC_LONG_MAX ((CC_LONG)-1)
20-
21-
#define git_hash_ctx_init(ctx) git_hash_init(ctx)
22-
#define git_hash_ctx_cleanup(ctx)
23-
24-
GIT_INLINE(int) git_hash_global_init(void)
25-
{
26-
return 0;
27-
}
28-
29-
GIT_INLINE(int) git_hash_init(git_hash_ctx *ctx)
30-
{
31-
assert(ctx);
32-
CC_SHA1_Init(&ctx->c);
33-
return 0;
34-
}
35-
36-
GIT_INLINE(int) git_hash_update(git_hash_ctx *ctx, const void *_data, size_t len)
37-
{
38-
const unsigned char *data = _data;
39-
40-
assert(ctx);
41-
42-
while (len > 0) {
43-
CC_LONG chunk = (len > CC_LONG_MAX) ? CC_LONG_MAX : (CC_LONG)len;
44-
45-
CC_SHA1_Update(&ctx->c, data, chunk);
46-
47-
data += chunk;
48-
len -= chunk;
49-
}
50-
51-
return 0;
52-
}
53-
54-
GIT_INLINE(int) git_hash_final(git_oid *out, git_hash_ctx *ctx)
55-
{
56-
assert(ctx);
57-
CC_SHA1_Final(out->id, &ctx->c);
58-
return 0;
59-
}
60-
6119
#endif

‎src/hash/hash_generic.c‎

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,6 @@
77

88
#include "hash_generic.h"
99

10-
#include "hash.h"
11-
1210
#if defined(__GNUC__) && (defined(__i386__) || defined(__x86_64__))
1311

1412
/*
@@ -221,6 +219,21 @@ static void hash__block(git_hash_ctx *ctx, const unsigned int *data)
221219
ctx->H[4] += E;
222220
}
223221

222+
int git_hash_global_init(void)
223+
{
224+
return 0;
225+
}
226+
227+
int git_hash_ctx_init(git_hash_ctx *ctx)
228+
{
229+
return git_hash_init(ctx);
230+
}
231+
232+
void git_hash_ctx_cleanup(git_hash_ctx *ctx)
233+
{
234+
GIT_UNUSED(ctx);
235+
}
236+
224237
int git_hash_init(git_hash_ctx *ctx)
225238
{
226239
ctx->size = 0;
@@ -285,4 +298,3 @@ int git_hash_final(git_oid *out, git_hash_ctx *ctx)
285298

286299
return 0;
287300
}
288-

‎src/hash/hash_generic.h‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,6 @@
88
#ifndef INCLUDE_hash_hash_generic_h__
99
#define INCLUDE_hash_hash_generic_h__
1010

11-
#include "common.h"
12-
1311
#include "hash.h"
1412

1513
struct git_hash_ctx {
@@ -18,12 +16,4 @@ struct git_hash_ctx {
1816
unsigned int W[16];
1917
};
2018

21-
#define git_hash_ctx_init(ctx) git_hash_init(ctx)
22-
#define git_hash_ctx_cleanup(ctx)
23-
24-
GIT_INLINE(int) git_hash_global_init(void)
25-
{
26-
return 0;
27-
}
28-
2919
#endif

‎src/hash/hash_mbedtls.c‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,16 @@
99
#include "hash.h"
1010
#include "hash/hash_mbedtls.h"
1111

12+
int git_hash_global_init(void)
13+
{
14+
return 0;
15+
}
16+
17+
int git_hash_ctx_init(git_hash_ctx *ctx)
18+
{
19+
return git_hash_init(ctx);
20+
}
21+
1222
void git_hash_ctx_cleanup(git_hash_ctx *ctx)
1323
{
1424
assert(ctx);

‎src/hash/hash_mbedtls.h‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,17 +8,12 @@
88
#ifndef INCLUDE_hash_mbedtld_h__
99
#define INCLUDE_hash_mbedtld_h__
1010

11+
#include "hash.h"
12+
1113
#include <mbedtls/sha1.h>
1214

1315
struct git_hash_ctx {
1416
mbedtls_sha1_context c;
1517
};
1618

17-
#define git_hash_ctx_init(ctx) git_hash_init(ctx)
18-
19-
GIT_INLINE(int) git_hash_global_init(void)
20-
{
21-
return 0;
22-
}
23-
2419
#endif /* INCLUDE_hash_mbedtld_h__ */

0 commit comments

Comments
 (0)