Re: [PATCH v2 1/2] read-cache: factor out get_sha1_from_index() helper

[Duy Nguyen]

On Wed, Oct 12, 2016 at 8:47 PM,   wrote:
> From: Torsten Bögershausen 
>
> Factor out the retrieval of the sha1 for a given path in
> read_blob_data_from_index() into the function get_sha1_from_index().
>
> This will be used in the next commit, when convert.c can do the
> analyze for "text=auto" without slurping the whole blob into memory
> at once.
>
> Add a wrapper definition get_sha1_from_cache().
>
> Signed-off-by: Torsten Bögershausen 
> ---
>  cache.h  |  3 +++
>  read-cache.c | 29 ++---
>  2 files changed, 21 insertions(+), 11 deletions(-)
>
> diff --git a/cache.h b/cache.h
> index 1604e29..04de209 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -380,6 +380,7 @@ extern void free_name_hash(struct index_state *istate);
>  #define unmerge_cache_entry_at(at) unmerge_index_entry_at(_index, at)
>  #define unmerge_cache(pathspec) unmerge_index(_index, pathspec)
>  #define read_blob_data_from_cache(path, sz) 
> read_blob_data_from_index(_index, (path), (sz))
> +#define get_sha1_from_cache(path)  get_sha1_from_index (_index, (path))
>  #endif
>
>  enum object_type {
> @@ -1089,6 +1090,8 @@ static inline void *read_sha1_file(const unsigned char 
> *sha1, enum object_type *
> return read_sha1_file_extended(sha1, type, size, 
> LOOKUP_REPLACE_OBJECT);
>  }
>
> +const unsigned char *get_sha1_from_index(struct index_state *istate, const 
> char *path);
> +
>  /*
>   * This internal function is only declared here for the benefit of
>   * lookup_replace_object().  Please do not call it directly.
> diff --git a/read-cache.c b/read-cache.c
> index 38d67fa..5a1df14 100644
> --- a/read-cache.c
> +++ b/read-cache.c
> @@ -2290,13 +2290,27 @@ int index_name_is_other(const struct index_state 
> *istate, const char *name,
>
>  void *read_blob_data_from_index(struct index_state *istate, const char 
> *path, unsigned long *size)
>  {
> -   int pos, len;
> +   const unsigned char *sha1;
> unsigned long sz;
> enum object_type type;
> void *data;
>
> -   len = strlen(path);
> -   pos = index_name_pos(istate, path, len);
> +   sha1 = get_sha1_from_index(istate, path);
> +   if (!sha1)
> +   return NULL;
> +   data = read_sha1_file(sha1, , );
> +   if (!data || type != OBJ_BLOB) {
> +   free(data);
> +   return NULL;
> +   }
> +   if (size)
> +   *size = sz;
> +   return data;
> +}
> +
> +const unsigned char *get_sha1_from_index(struct index_state *istate, const 
> char *path)

Let's try to embrace struct object_id to make our lives easier when
the time comes to convert to a new hash algorithm by returning struct
object_id * here instead of the internal hash.

> +{
> +   int pos = index_name_pos(istate, path, strlen(path));
> if (pos < 0) {
> /*
>  * We might be in the middle of a merge, in which
> @@ -2312,14 +2326,7 @@ void *read_blob_data_from_index(struct index_state 
> *istate, const char *path, un
> }
> if (pos < 0)
> return NULL;
> -   data = read_sha1_file(istate->cache[pos]->oid.hash, , );
> -   if (!data || type != OBJ_BLOB) {
> -   free(data);
> -   return NULL;
> -   }
> -   if (size)
> -   *size = sz;
> -   return data;
> +   return istate->cache[pos]->oid.hash;
>  }
>
>  void stat_validity_clear(struct stat_validity *sv)
-- 
Duy

Re: [PATCH v2 1/2] read-cache: factor out get_sha1_from_index() helper

[Junio C Hamano]

tbo...@web.de writes:

> From: Torsten Bögershausen 
>
> Factor out the retrieval of the sha1 for a given path in
> read_blob_data_from_index() into the function get_sha1_from_index().
>
> This will be used in the next commit, when convert.c can do the
> analyze for "text=auto" without slurping the whole blob into memory
> at once.
>
> Add a wrapper definition get_sha1_from_cache().
>
> Signed-off-by: Torsten Bögershausen 
> ---
>  cache.h  |  3 +++
>  read-cache.c | 29 ++---
>  2 files changed, 21 insertions(+), 11 deletions(-)
>
> diff --git a/cache.h b/cache.h
> index 1604e29..04de209 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -380,6 +380,7 @@ extern void free_name_hash(struct index_state *istate);
>  #define unmerge_cache_entry_at(at) unmerge_index_entry_at(_index, at)
>  #define unmerge_cache(pathspec) unmerge_index(_index, pathspec)
>  #define read_blob_data_from_cache(path, sz) 
> read_blob_data_from_index(_index, (path), (sz))
> +#define get_sha1_from_cache(path)  get_sha1_from_index (_index, (path))

Should have caught this earlier, but there is an extra SP after "from_index"
which I'll remove (the topic is not in 'next' yet, lucky us).

I re-read this to ensure that it does not break read_blob_data_from_index()
the new function borrows the logic from.

Thanks.

[PATCH v2 1/2] read-cache: factor out get_sha1_from_index() helper

[tboegi]

From: Torsten Bögershausen 

Factor out the retrieval of the sha1 for a given path in
read_blob_data_from_index() into the function get_sha1_from_index().

This will be used in the next commit, when convert.c can do the
analyze for "text=auto" without slurping the whole blob into memory
at once.

Add a wrapper definition get_sha1_from_cache().

Signed-off-by: Torsten Bögershausen 
---
 cache.h  |  3 +++
 read-cache.c | 29 ++---
 2 files changed, 21 insertions(+), 11 deletions(-)

diff --git a/cache.h b/cache.h
index 1604e29..04de209 100644
--- a/cache.h
+++ b/cache.h
@@ -380,6 +380,7 @@ extern void free_name_hash(struct index_state *istate);
 #define unmerge_cache_entry_at(at) unmerge_index_entry_at(_index, at)
 #define unmerge_cache(pathspec) unmerge_index(_index, pathspec)
 #define read_blob_data_from_cache(path, sz) 
read_blob_data_from_index(_index, (path), (sz))
+#define get_sha1_from_cache(path)  get_sha1_from_index (_index, (path))
 #endif
 
 enum object_type {
@@ -1089,6 +1090,8 @@ static inline void *read_sha1_file(const unsigned char 
*sha1, enum object_type *
return read_sha1_file_extended(sha1, type, size, LOOKUP_REPLACE_OBJECT);
 }
 
+const unsigned char *get_sha1_from_index(struct index_state *istate, const 
char *path);
+
 /*
  * This internal function is only declared here for the benefit of
  * lookup_replace_object().  Please do not call it directly.
diff --git a/read-cache.c b/read-cache.c
index 38d67fa..5a1df14 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -2290,13 +2290,27 @@ int index_name_is_other(const struct index_state 
*istate, const char *name,
 
 void *read_blob_data_from_index(struct index_state *istate, const char *path, 
unsigned long *size)
 {
-   int pos, len;
+   const unsigned char *sha1;
unsigned long sz;
enum object_type type;
void *data;
 
-   len = strlen(path);
-   pos = index_name_pos(istate, path, len);
+   sha1 = get_sha1_from_index(istate, path);
+   if (!sha1)
+   return NULL;
+   data = read_sha1_file(sha1, , );
+   if (!data || type != OBJ_BLOB) {
+   free(data);
+   return NULL;
+   }
+   if (size)
+   *size = sz;
+   return data;
+}
+
+const unsigned char *get_sha1_from_index(struct index_state *istate, const 
char *path)
+{
+   int pos = index_name_pos(istate, path, strlen(path));
if (pos < 0) {
/*
 * We might be in the middle of a merge, in which
@@ -2312,14 +2326,7 @@ void *read_blob_data_from_index(struct index_state 
*istate, const char *path, un
}
if (pos < 0)
return NULL;
-   data = read_sha1_file(istate->cache[pos]->oid.hash, , );
-   if (!data || type != OBJ_BLOB) {
-   free(data);
-   return NULL;
-   }
-   if (size)
-   *size = sz;
-   return data;
+   return istate->cache[pos]->oid.hash;
 }
 
 void stat_validity_clear(struct stat_validity *sv)
-- 
2.10.0