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

2015-03-04 Thread Junio C Hamano 
Karthik Nayak karthik@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@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...

 +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.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2015-03-04 Thread karthik nayak 


On 03/05/2015 02:28 AM, Junio C Hamano wrote:

Karthik Nayak karthik@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@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.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2015-03-04 Thread Junio C Hamano 
karthik nayak karthik@gmail.com writes:

 Looks suspiciously familiar...
 Yes, you suggested it.
 It has a similar structure to unpack_sha1_header().
 Anything wrong with it?

I don't know if there is something wrong with the code, or not, but
it wasn't mentioned in the log message at all that it is not your
code, and I was mostly reacting to that.

It is fairly common around here to take somebody else's code and run
with it, but people say things like This is based on suggestion
from X, This function was stolen from Y, etc. and then conclude
with but I adjusted it to match the caller I wrote, so bugs are
mine. when they do so.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2015-03-03 Thread Karthik Nayak 
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@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)
+{
+   unsigned char buffer[32], *cp;
+   unsigned long bufsiz = sizeof(buffer);
+   int status;
+
+   /* Get the data stream */
+   memset(stream, 0, sizeof(*stream));
+   stream-next_in = map;
+   stream-avail_in = mapsize;
+   stream-next_out = buffer;
+   stream-avail_out = bufsiz;
+
+   git_inflate_init(stream);
+
+   do {
+   status = git_inflate(stream, 0);
+   strbuf_add(header, buffer, stream-next_out - buffer);
+   for (cp = buffer; cp  stream-next_out; cp++)
+   if (!*cp)
+   /* Found the NUL at the end of the header */
+   return 0;
+   stream-next_out = buffer;
+   stream-avail_out = bufsiz;
+   } while (status == Z_OK);
+   return -1;
+}
+
 static void *unpack_sha1_rest(git_zstream *stream, void *buffer, unsigned long 
size, const unsigned char *sha1)
 {
int bytes = strlen(buffer) + 1;
@@ -2524,13 +2554,16 @@ struct packed_git *find_sha1_pack(const unsigned char 
*sha1,
 }
 
 static int sha1_loose_object_info(const unsigned char *sha1,
- struct object_info *oi)
+ struct object_info *oi,
+ int flags)
 {
-   int status;
+   int status = 0;
unsigned long mapsize, size;
void *map;
git_zstream stream;
char hdr[32];
+   struct strbuf hdrbuf = STRBUF_INIT;
+   char *hdrp;
 
if (oi-delta_base_sha1)
hashclr(oi-delta_base_sha1);
@@ -2557,10 +2590,25 @@ static int sha1_loose_object_info(const unsigned char 
*sha1,
return -1;
if (oi-disk_sizep)
*oi-disk_sizep = mapsize;
-   if (unpack_sha1_header(stream, map, mapsize, hdr, sizeof(hdr))  0)
-   status = error(unable to unpack %s header,
-  sha1_to_hex(sha1));
-   else if ((status = parse_sha1_header(hdr, size))  0)
+   if ((flags  LOOKUP_LITERALLY)) {
+   if (unpack_sha1_header_literally(stream, map, mapsize, 
hdrbuf)  0)
+   status = error(unable to unpack %s header with 
--literally,
+  sha1_to_hex(sha1));
+   hdrp = hdrbuf.buf;
+   } else {
+   if (unpack_sha1_header(stream, map, mapsize, hdr, sizeof(hdr)) 
 0) {
+   status = error(unable to unpack %s header,
+  sha1_to_hex(sha1));
+   }
+   hdrp = hdr;
+   }
+   if (status)
+   ; /* We're already checking for errors */
+   else if ((flags  LOOKUP_LITERALLY)) {
+   size_t typelen = strcspn(hdrbuf.buf,  );
+   strbuf_add(oi-typename, hdrbuf.buf, typelen);
+   }
+   else if ((status = parse_sha1_header(hdrp, size))  0)
status = error(unable to parse %s header, sha1_to_hex(sha1));
else if (oi-sizep)
*oi-sizep = size;
@@ -2568,6 +2616,10 @@ static int sha1_loose_object_info(const unsigned char 
*sha1,
munmap(map, mapsize);
if (oi-typep)
*oi-typep = status;
+   if (oi-typename  0 = status  typename(status))
+   strbuf_addstr(oi-typename, typename(status));
+   if (hdrp == hdrbuf.buf)
+   strbuf_release(hdrbuf);
return 0;
 }
 
@@ -2594,7 +2646,7 @@ int sha1_object_info_extended(const unsigned char *sha1, 
struct object_info *oi,
 
if (!find_pack_entry(real, e)) {
/* Most likely it's a loose object. */
-   if