Re: [PATCH v2 2/3] sha1_file: implement changes for "cat-file --literally -t"

[karthik nayak <karthik.188@gmail.com>]

On 03/05/2015 02:28 AM, Junio C Hamano wrote:
> Karthik Nayak <karthik.188@gmail.com> writes:
>
> > add "sha1_object_info_literally()" which is to be used when
> > the "literally" option is given to get the type of object
> > and print it, using "sha1_object_info_extended()".
> >
> > add "unpack_sha1_header_literally()" to unpack sha headers
> > which may be greater than 32 bytes, which is the threshold
> > for a regular object header.
> >
> > modify "sha1_loose_object_info()" to include a flag argument
> > and also include changes to call "unpack_sha1_header_literally()"
> > when the literally flag is passed. Also copies the obtained
> > type to the typename field of object_info.
> >
> > modify "sha1_object_info_extended()" to call the function
> > "sha1_loose_object_info()" with flags.
> >
> > Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
> > ---
> >   sha1_file.c | 84 +++++++++++++++++++++++++++++++++++++++++++++++++++++++------
> >   1 file changed, 77 insertions(+), 7 deletions(-)
> >
> > diff --git a/sha1_file.c b/sha1_file.c
> > index 69a60ec..1068ca0 100644
> > --- a/sha1_file.c
> > +++ b/sha1_file.c
> > @@ -1564,6 +1564,36 @@ int unpack_sha1_header(git_zstream *stream, unsigned char *map, unsigned long ma
> >       return git_inflate(stream, 0);
> >   }
> >
> > +static int unpack_sha1_header_literally(git_zstream *stream, unsigned char *map,
> > +                    unsigned long mapsize,
> > +                    struct strbuf *header)
> > +{
> > +...
> > +}
>
> Looks suspiciously familiar...
Yes, you suggested it.
It has a similar structure to unpack_sha1_header().
Anything wrong with it?
>
> > +int sha1_object_info_literally(const unsigned char *sha1)
> > +{
> > +    enum object_type type;
> > +    struct strbuf sb = STRBUF_INIT;
> > +    struct object_info oi = {NULL};
> > +
> > +    oi.typename = &sb;
> > +    oi.typep = &type;
> > +    if (sha1_object_info_extended(sha1, &oi, LOOKUP_LITERALLY) < 0)
> > +        return -1;
> > +    if (*oi.typep > 0)
> > +        printf("%s\n", typename(*oi.typep));
> > +    else
> > +        printf("%s\n", oi.typename->buf);
> > +    strbuf_release(oi.typename);
> > +    return 0;
> > +}
> > +
>
> NAK.
>
> Please don't add end-user facing final output to sha1_file.c;
> instead have the caller use a helper function like this one to
> extract necessary information and perform the end-user interaction
> there.
>
Ok, will do that.