patch, 15 partsRearrange xcalloc arguments

xcalloc takes two arguments: the number of elements and their size. The vast majority of the Git codebase passes these arguments in the correct order, but there are some exceptions. This patch series corrects those exceptions.

Brian Gesiak (15):
  builtin/add.c: rearrange xcalloc arguments
  builtin/ls-remote.c: rearrange xcalloc arguments
  builtin/remote.c: rearrange xcalloc arguments
  commit.c: rearrange xcalloc arguments
  config.c: rearrange xcalloc arguments
  diff.c: rearrange xcalloc arguments
  hash.c: rearrange xcalloc arguments
  hash.h: rearrange xcalloc arguments
  http-push.c: rearrange xcalloc arguments
  imap-send.c: rearrange xcalloc arguments
  notes.c: rearrange xcalloc arguments
  pack-revindex.c: rearrange xcalloc arguments
  reflog-walk.c: rearrange xcalloc arguments
  remote.c: rearrange xcalloc arguments
  transport-helper.c: rearrange xcalloc arguments

builtin/add.c | 2 +- builtin/ls-remote.c | 2 +- builtin/remote.c | 8 ++++---- commit.c | 2 +- config.c | 4 ++-- diff.c | 2 +- hash.c | 2 +- hash.h | 2 +- http-push.c | 2 +- imap-send.c | 2 +- notes.c | 6 +++--- pack-revindex.c | 2 +- reflog-walk.c | 8 ++++---- remote.c | 2 +- transport-helper.c | 2 +- 15 files changed, 24 insertions(+), 24 deletions(-)

xcalloc takes two arguments: the number of elements and their size. run_add_interactive passes the arguments in reverse order, passing the size of a char*, followed by the number of char* to be allocated. Rearrgange them so they are in the correct order.

Signed-off-by: Brian Gesiak <modocache@gmail.com>
---
 builtin/add.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Brian,

On Tue, May 27, 2014 at 12:33:42AM +0900, Brian Gesiak wrote:

This patch doesn't apply to any of the branches I have available (master, pu, next). And there is no line containing "pathspec->nr + 6" anywhere in my builtin/add.c. Which branch is your work based off?

My apologies! I based my work off of maint, branching off of eea591.

My reasoning was that Documentation/SubmittingPatches states that "a bugfix should be based on 'maint'". [1] Now that I think about it, this is probably not the kind of "bug" that statement had in mind.

Should I reroll the patch based on master?

- Brian Gesiak

[1] https://github.com/git/git/blob/4a28f169ad29ba452e0e7bea2583914c10c58322/Documentation/SubmittingPatches#L9

On Tue, May 27, 2014 at 8:11 AM, Jeremiah Mahler <jmmahler@gmail.com> wrote:

On Tue, May 27, 2014 at 11:22:00AM +0900, Brian Gesiak wrote:

OK, got it. I should have read Documentation/SubmittingPatches more closely like you did :-) No need to reroll I can just use the maint branch to test it out. Thanks!

On Mon, May 26, 2014 at 11:33 AM, Brian Gesiak <modocache@gmail.com> wrote:

s/Rearrgange/Rearrange/

Same misspelling afflicts the entire patch series.

Oomph, how embarrassing. Thanks for pointing that out!

Would it be better if I rerolled the patches?

- Brian Gesiak

On Tue, May 27, 2014 at 12:25 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:

On Tue, May 27, 2014 at 7:32 AM, Brian Gesiak <modocache@gmail.com> wrote:

Etiquette on this list is to avoid top-posting [1].

[1]: https://lkml.org/lkml/2005/1/11/111

Junio may or may not make small fixes himself when he picks up a patch series. If you don't hear from him and your patches don't appear in his 'pu' branch with that fix, re-rolling might be advisable.

If you do re-roll, perhaps consider simplifying the commit messages. The patch itself states concisely and precisely what is being changed; the lengthy prose description doesn't really add anything (and makes more work for you and the reader of the message). It might be sufficient to use a single-line (Subject:) commit message, like this:

    builtin/add.c: fix order of xcalloc arguments

Eric Sunshine <sunshine@sunshineco.com> writes:

Yeah, I like that.

I do not think it is worth doing this change starting from maint, so I've dropped this one and a few others that did not apply to master and queued the remainder to 'pu'.

On Tue, May 27, 2014 at 05:35:29PM -0400, Eric Sunshine wrote:

A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

That is the funniest post I have ever seen by Kroah.

On Wed, May 28, 2014 at 7:41 AM, Junio C Hamano <gitster@pobox.com> wrote:

Thank you! I'll keep this in mind when choosing what to branch off of in the future.

On Wed, May 28, 2014 at 6:35 AM, Eric Sunshine <sunshine@sunshineco.com> wrote:

Thank you for the tips; very much appreciated.

- Brian Gesiak

xcalloc takes two arguments: the number of elements and their size. cmd_ls_remote passes the arguments in reverse order, passing the size of a char*, followed by the number of char* to be allocated. Rearrgange them so they are in the correct order.

Signed-off-by: Brian Gesiak <modocache@gmail.com>
---
 builtin/ls-remote.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

xcalloc takes two arguments: the number of elements and their size. builtin/remote.c includes several calls to xcalloc that pass the arguments in reverse order. Rearrgange them so they are in the correct order.

Signed-off-by: Brian Gesiak <modocache@gmail.com>
---
 builtin/remote.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

xcalloc takes two arguments: the number of elements and their size. reduce_heads passes the arguments in reverse order, passing the size of a commit*, followed by the number of commit* to be allocated. Rearrgange them so they are in the correct order.

Signed-off-by: Brian Gesiak <modocache@gmail.com>
---
 commit.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

xcalloc takes two arguments: the number of elements and their size. config.c includes several calls to xcalloc that pass the arguments in reverse order: the size of a struct lock_file*, followed by the number to allocate. Rearrgange them so they are in the correct order.

Signed-off-by: Brian Gesiak <modocache@gmail.com>
---
 config.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

xcalloc takes two arguments: the number of elements and their size. diffstat_add passes the arguments in reverse order, passing the size of a diffstat_file*, followed by the number of diffstat_file* to be allocated. Rearrgange them so they are in the correct order.

Signed-off-by: Brian Gesiak <modocache@gmail.com>
---
 diff.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

xcalloc takes two arguments: the number of elements and their size. grow_hash_table passes the arguments in reverse order, passing the size of a hash table entry, followed by the number of entries. Rearrgange them so they are in the correct order.

Signed-off-by: Brian Gesiak <modocache@gmail.com>
---
 hash.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

xcalloc takes two arguments: the number of elements and their size. prellocate_hash passes the arguments in reverse order, passing the size of a hash table entry, followed by the number of entries. Rearrgange them so they are in the correct order.

Signed-off-by: Brian Gesiak <modocache@gmail.com>
---
 hash.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

xcalloc takes two arguments: the number of elements and their size. http-push passes the arguments in reverse order, passing the size of a repo, followed by the number to allocate. Rearrgange them so they are in the correct order.

Signed-off-by: Brian Gesiak <modocache@gmail.com>
---
 http-push.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

xcalloc takes two arguments: the number of elements and their size. imap_open_store passes the arguments in reverse order, passing the size of an imap_store*, followed by the number to allocate. Rearrgange them so they are in the correct order.

Signed-off-by: Brian Gesiak <modocache@gmail.com>
---
 imap-send.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Brian,

On Tue, May 27, 2014 at 12:33:51AM +0900, Brian Gesiak wrote:

                       ^^^^^^^^^^^^^^^^^^^^^^^^

Why wasn't the second instance swapped? It is still this way in the latest 'pu' branch.

xcalloc takes two arguments: the number of elements and their size. notes.c includes several calls to xcalloc that pass the arguments in reverse order. Rearrgange them so they are in the correct order.

Signed-off-by: Brian Gesiak <modocache@gmail.com>
---
 notes.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

xcalloc takes two arguments: the number of elements and their size. init_pack_revindex passes the arguments in reverse order, passing the size of a pack_revindex, followed by the number to allocate. Rearrgange them so they are in the correct order.

Signed-off-by: Brian Gesiak <modocache@gmail.com>
---
 pack-revindex.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

xcalloc takes two arguments: the number of elements and their size. reflog-walk.c includes several calls to xcalloc that pass the arguments in reverse order. Rearrgange them so they are in the correct order.

Signed-off-by: Brian Gesiak <modocache@gmail.com>
---
 reflog-walk.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

xcalloc takes two arguments: the number of elements and their size. parse_refspec_internal passes the arguments in reverse order, passing the size of a refspec, followed by the number to allocate. Rearrgange them so they are in the correct order.

Signed-off-by: Brian Gesiak <modocache@gmail.com>
---
 remote.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

xcalloc takes two arguments: the number of elements and their size. transport_helper_init passes the arguments in reverse order, passing the size of a helper_data*, followed by the number to allocate. Rearrgange them so they are in the correct order.

Signed-off-by: Brian Gesiak <modocache@gmail.com>
---
 transport-helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Brian,

On Tue, May 27, 2014 at 12:33:41AM +0900, Brian Gesiak wrote:

Let me see if I understand the issue underlying this patch set.

xcalloc works like calloc and takes two arguments, the number of elements and the size of each element. However, many calls specified these arguments in the reverse order. It didn't produce a compile error because both arguments are the same type. And it didn't produce a run time error because A*B is the same as B*A.

If this behaved like dd, performance would be different depending on the order.

  dd if=in of=out bs=1    count=1024
  dd if=in of=out bs=1024 count=1

Nonetheless, it appears to be a good fix.  Nice job!

On Mon, May 26, 2014 at 04:37:41PM -0700, Jeremiah Mahler wrote:

Yes, I think that is a good summary.

It may be theoretically possible that an implementation of calloc() can use the distinction between the two arguments to adjust the padding or alignment of the result. However, I don't know if any implementation actually does this, or if it is even true in theory. You can find some discussions[1,2] online, but nothing conclusive. The most plausible theory I saw is that early K&R C may have done something clever here, but ANSI C alignment requirements effectively remove any wiggle room for the implementation.

But it certainly does not hurt to follow the spec and be consistent.

-Peff

[1] http://stackoverflow.com/questions/501839/is-calloc4-6-the-same-as-calloc6-4

[2] https://groups.google.com/d/msg/comp.lang.c/jZbiyuYqjB4/NIAmeNd11IoJ