From: Lukas Fleischer <git@cryptocrack.de>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH v2 1/3] Add public function read_blob_data_from_index_path()
Date: Sun, 14 Apr 2013 22:14:15 +0200 [thread overview]
Message-ID: <20130414201415.GA9683@blizzard> (raw)
In-Reply-To: <7vwqs56418.fsf@alter.siamese.dyndns.org>
On Sat, Apr 13, 2013 at 10:49:36PM -0700, Junio C Hamano wrote:
> Thanks. I'll queue with the following API fix-up on this, with
> obvious adjustments necessary for the later ones.
>
> * read_blob_data_from_index() is descriptive enough. If you read a
> blob from the index, you would ask for it with a path; there is
> no other sensible key to do so.
>
> * A function in read-cache API that works on an index should have
> the istate as the first argument and called with name "index";
>
> * If callers want to operate on the default index, macro with a
> similar name but replacing "index" with "cache" can be supplied,
> e.g.
>
> #define distim_cache(x) distim_index(&the_index, x)
> ---
> attr.c | 2 +-
> cache.h | 3 ++-
> read-cache.c | 4 +---
> 3 files changed, 4 insertions(+), 5 deletions(-)
Sounds reasonable to me -- squashing the fixed up version into my
original patch ([PATCH v2 1/1] Add public function
read_blob_data_from_index_path()) probably makes sense here :)
Thanks for reviewing and reworking!
>
> diff --git a/attr.c b/attr.c
> index a668a63..46bde57 100644
> --- a/attr.c
> +++ b/attr.c
> @@ -387,7 +387,7 @@ static struct attr_stack *read_attr_from_index(const char *path, int macro_ok)
> char *buf, *sp;
> int lineno = 0;
>
> - buf = read_blob_data_from_index_path(path, use_index);
> + buf = read_blob_data_from_index(use_index, path);
> if (!buf)
> return NULL;
>
> diff --git a/cache.h b/cache.h
> index 85ddb13..5d66c74 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -307,6 +307,7 @@ extern void free_name_hash(struct index_state *istate);
> #define resolve_undo_clear() resolve_undo_clear_index(&the_index)
> #define unmerge_cache_entry_at(at) unmerge_index_entry_at(&the_index, at)
> #define unmerge_cache(pathspec) unmerge_index(&the_index, pathspec)
> +#define read_blob_data_from_cache(path) read_blob_data_from_index(&the_index, (path))
> #endif
>
> enum object_type {
> @@ -452,7 +453,7 @@ extern int add_file_to_index(struct index_state *, const char *path, int flags);
> extern struct cache_entry *make_cache_entry(unsigned int mode, const unsigned char *sha1, const char *path, int stage, int refresh);
> extern int ce_same_name(struct cache_entry *a, struct cache_entry *b);
> extern int index_name_is_other(const struct index_state *, const char *, int);
> -extern void *read_blob_data_from_index_path(const char *path, struct index_state *use_index);
> +extern void *read_blob_data_from_index(struct index_state *, const char *);
>
> /* do stat comparison even if CE_VALID is true */
> #define CE_MATCH_IGNORE_VALID 01
> diff --git a/read-cache.c b/read-cache.c
> index cbeb248..48d87e8 100644
> --- a/read-cache.c
> +++ b/read-cache.c
> @@ -1896,14 +1896,12 @@ int index_name_is_other(const struct index_state *istate, const char *name,
> return 1;
> }
>
> -void *read_blob_data_from_index_path(const char *path,
> - struct index_state *use_index)
> +void *read_blob_data_from_index(struct index_state *istate, const char *path)
> {
> int pos, len;
> unsigned long sz;
> enum object_type type;
> void *data;
> - struct index_state *istate = use_index ? use_index : &the_index;
>
> len = strlen(path);
> pos = index_name_pos(istate, path, len);
> --
> 1.8.2.1-514-gf369d36
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
prev parent reply other threads:[~2013-04-14 20:14 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-04-12 17:26 [PATCH 0/3] Remove ~25 lines of duplicate code Lukas Fleischer
2013-04-12 17:26 ` [PATCH 1/3] Make read_index_data() public Lukas Fleischer
2013-04-12 19:40 ` Jeff King
2013-04-13 8:23 ` Lukas Fleischer
2013-04-12 17:26 ` [PATCH 2/3] Add size parameter to read_index_data() Lukas Fleischer
2013-04-12 17:26 ` [PATCH 3/3] convert.c: Remove duplicate code Lukas Fleischer
2013-04-13 13:28 ` [PATCH v2 1/3] Add public function read_blob_data_from_index_path() Lukas Fleischer
2013-04-13 13:28 ` [PATCH v2 2/3] Add size parameter to read_blob_data_from_index_path() Lukas Fleischer
2013-04-13 13:28 ` [PATCH v2 3/3] convert.c: Remove duplicate code Lukas Fleischer
2013-04-14 5:49 ` [PATCH v2 1/3] Add public function read_blob_data_from_index_path() Junio C Hamano
2013-04-14 6:55 ` Junio C Hamano
2013-04-14 20:14 ` Lukas Fleischer [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20130414201415.GA9683@blizzard \
--to=git@cryptocrack.de \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).