Re: [WIP/PATCH v5 05/10] for-each-ref: introduce 'ref_array_clear()'

2015-06-09 Thread Matthieu Moy
Karthik Nayak karthik@gmail.com writes:

 On 06/08/2015 10:51 PM, Matthieu Moy wrote:

 We could introduce ref-filter.h earlier, indeed. To me, the current
 solution is good enough, but introducing ref-filter.h early and adding
 function definition there in the same commit as you drop the static
 keyword for them would clearly be an improvement.

 But that would break the flow, wouldn't it? I wanted ref-filter to be
 introduced together, hence right after ref-filter.h we move code to
 ref-filter.c

That's why I find the current solution good enough: it also has
advantages. But in the current series, when you say make functions
public, you are not actually doing so since they're not exported in a
.h file.

Conversely, PATCH 07 does two things: move code from for-each-ref.c and
introduce new declarations. Had you introduced these declarations
earlier, this patch would have been pure code movement.

In both cases, you have intermediate states that are not fully
consistant: either you have public functions in the builtin/ directory
(which sometimes happen in Git's codebase, but we try to avoid it), or
you have non-static functions that are not declared in a .h.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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: [WIP/PATCH v5 05/10] for-each-ref: introduce 'ref_array_clear()'

2015-06-08 Thread Karthik Nayak

On 06/08/2015 10:51 PM, Matthieu Moy wrote:

Junio C Hamano gits...@pobox.com writes:


Matthieu Moy matthieu@grenoble-inp.fr writes:


Karthik Nayak karthik@gmail.com writes:


On 06/08/2015 08:23 PM, Matthieu Moy wrote:

Karthik Nayak karthik@gmail.com writes:


+/* Free all memory allocated for ref_array */
+void ref_array_clear(struct ref_array *array)


Is this a private function? If so, then add static. If not, you probably
want to export it in a .h file.


It is in ref-filter.h.


Ah, OK. It comes later in the series.


Confused I am; if it comes later not in the same patch then it is
not OK, is it?


We could introduce ref-filter.h earlier, indeed. To me, the current
solution is good enough, but introducing ref-filter.h early and adding
function definition there in the same commit as you drop the static
keyword for them would clearly be an improvement.



But that would break the flow, wouldn't it? I wanted ref-filter to be 
introduced together, hence right after ref-filter.h we move code to

ref-filter.c

--
Regards,
Karthik
--
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: [WIP/PATCH v5 05/10] for-each-ref: introduce 'ref_array_clear()'

2015-06-08 Thread Junio C Hamano
Matthieu Moy matthieu@grenoble-inp.fr writes:

 Karthik Nayak karthik@gmail.com writes:

 On 06/08/2015 08:23 PM, Matthieu Moy wrote:
 Karthik Nayak karthik@gmail.com writes:

  +/* Free all memory allocated for ref_array */
  +void ref_array_clear(struct ref_array *array)

 Is this a private function? If so, then add static. If not, you probably
 want to export it in a .h file.

 It is in ref-filter.h.

 Ah, OK. It comes later in the series.

Confused I am; if it comes later not in the same patch then it is
not OK, is it?
--
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: [WIP/PATCH v5 05/10] for-each-ref: introduce 'ref_array_clear()'

2015-06-08 Thread Matthieu Moy
Junio C Hamano gits...@pobox.com writes:

 Matthieu Moy matthieu@grenoble-inp.fr writes:

 Karthik Nayak karthik@gmail.com writes:

 On 06/08/2015 08:23 PM, Matthieu Moy wrote:
 Karthik Nayak karthik@gmail.com writes:

  +/* Free all memory allocated for ref_array */
  +void ref_array_clear(struct ref_array *array)

 Is this a private function? If so, then add static. If not, you probably
 want to export it in a .h file.

 It is in ref-filter.h.

 Ah, OK. It comes later in the series.

 Confused I am; if it comes later not in the same patch then it is
 not OK, is it?

We could introduce ref-filter.h earlier, indeed. To me, the current
solution is good enough, but introducing ref-filter.h early and adding
function definition there in the same commit as you drop the static
keyword for them would clearly be an improvement.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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: [WIP/PATCH v5 05/10] for-each-ref: introduce 'ref_array_clear()'

2015-06-08 Thread Matthieu Moy
Karthik Nayak karthik@gmail.com writes:

 +/* Free all memory allocated for ref_array */
 +void ref_array_clear(struct ref_array *array)

Is this a private function? If so, then add static. If not, you probably
want to export it in a .h file.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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: [WIP/PATCH v5 05/10] for-each-ref: introduce 'ref_array_clear()'

2015-06-08 Thread Karthik Nayak

On 06/08/2015 08:23 PM, Matthieu Moy wrote:

Karthik Nayak karthik@gmail.com writes:

 +/* Free all memory allocated for ref_array */
 +void ref_array_clear(struct ref_array *array)

Is this a private function? If so, then add static. If not, you probably
want to export it in a .h file.


It is in ref-filter.h.

--
Regards,
Karthik
--
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: [WIP/PATCH v5 05/10] for-each-ref: introduce 'ref_array_clear()'

2015-06-08 Thread Karthik Nayak

On 06/08/2015 08:23 PM, Matthieu Moy wrote:

Karthik Nayak karthik@gmail.com writes:

 +/* Free all memory allocated for ref_array */
 +void ref_array_clear(struct ref_array *array)

Is this a private function? If so, then add static. If not, you probably
want to export it in a .h file.


It is in ref-filter.h.

--
Regards,
Karthik
--
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: [WIP/PATCH v5 05/10] for-each-ref: introduce 'ref_array_clear()'

2015-06-08 Thread Matthieu Moy
Karthik Nayak karthik@gmail.com writes:

 On 06/08/2015 08:23 PM, Matthieu Moy wrote:
 Karthik Nayak karthik@gmail.com writes:

  +/* Free all memory allocated for ref_array */
  +void ref_array_clear(struct ref_array *array)

 Is this a private function? If so, then add static. If not, you probably
 want to export it in a .h file.

 It is in ref-filter.h.

Ah, OK. It comes later in the series.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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


[WIP/PATCH v5 05/10] for-each-ref: introduce 'ref_array_clear()'

2015-06-06 Thread Karthik Nayak
Introduce and implement 'ref_array_clear()' which will free
all allocated memory for 'ref_array'.

Mentored-by: Christian Couder christian.cou...@gmail.com
Mentored-by: Matthieu Moy matthieu@grenoble-inp.fr
Signed-off-by: Karthik Nayak karthik@gmail.com
---
 builtin/for-each-ref.c | 21 +
 1 file changed, 21 insertions(+)

diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 169ccc8..54f03c9 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -911,6 +911,26 @@ static int grab_single_ref(const char *refname, const 
unsigned char *sha1, int f
return 0;
 }
 
+/*  Free memory allocated for a ref_array_item */
+static void free_array_item(struct ref_array_item *item)
+{
+   free(item-symref);
+   free(item-refname);
+   free(item);
+}
+
+/* Free all memory allocated for ref_array */
+void ref_array_clear(struct ref_array *array)
+{
+   int i;
+
+   for (i = 0; i  array-nr; i++)
+   free_array_item(array-items[i]);
+   free(array-items);
+   array-items = NULL;
+   array-nr = array-alloc = 0;
+}
+
 static int cmp_ref_sort(struct ref_sort *s, struct ref_array_item *a, struct 
ref_array_item *b)
 {
struct atom_value *va, *vb;
@@ -1141,5 +1161,6 @@ int cmd_for_each_ref(int argc, const char **argv, const 
char *prefix)
maxcount = ref_cbdata.array.nr;
for (i = 0; i  maxcount; i++)
show_ref(ref_cbdata.array.items[i], format, quote_style);
+   ref_array_clear(ref_cbdata.array);
return 0;
 }
-- 
2.4.2

--
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