{"thread":{"id":"20974","subject":"[PATCH v2] push: Correctly initialize nonfastforward in do_push.","startedAt":"2009-09-16T17:26:08Z","lastAt":"2009-09-16T18:29:21Z","messageCount":4,"participants":["Matthieu Moy","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"123363","messageId":"vpqfxamby33.fsf@bauges.imag.fr","threadId":"20974","inReplyTo":"7vpr9q24oa.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] push: Correctly initialize nonfastforward in do_push.","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2009-09-16T17:26:08Z","receivedAt":"2009-09-16T17:26:08Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Junio C Hamano writes:\n\n> Matthieu Moy writes:\n>\n>> The variable is assigned unconditionally in print_push_status, but\n>> print_push_status is not reached by all codepaths. In particular, this\n>> fixes a bug where \"git push ... nonexisting-branch\" was complaining about\n>> non-fast forward.\n>\n> Hmm, the patch looks correct but I am scratching my head to see how this\n> is triggered. \"git push ... nonexisting-branch\" seems to get:\n\nShort answer: trust me, without the patch, you get the non-fast\nforward (and valgrind tells you about conditional jump on\nuninitialized value), with, you don't ;-).\n\nLonger one:\n\nint transport_push(struct transport *transport,\n\t\t int refspec_nr, const char **refspec, int flags,\n\t\t int * nonfastforward)\n{\n[...]\n\t\tif (match_refs(local_refs, &remote_refs,\n\t\t\t refspec_nr, refspec, match_flags)) {\n\t\t\treturn -1; /* <------------------------------ you stop here */\n\t\t}\n[...]\n\t\tif (!quiet || push_had_errors(remote_refs))\n\t\t\tprint_push_status(transport->url, remote_refs,\n\t\t\t\t\tverbose | porcelain, porcelain,\n\t\t\t\t\tnonfastforward); /* <----- you would have updated nonfastforward there */\n[...]\n}\n\nActually, my initial version probably had the condition of the second\nif too. And with the first \"return\" statement in transport_push.\nWritting this, I'm wondering if it wouldn't be a better coding style\nto initialize nonfastforward to 0 within transport_push (in case other\ncallers to transport_push appear one day, they won't get the the same\nbug). Second version of the patch is comming.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"123362","messageId":"1253122107-31378-1-git-send-email-Matthieu.Moy@imag.fr","threadId":"20974","inReplyTo":"vpqfxamby33.fsf@bauges.imag.fr","subject":"[PATCH v2] push: Correctly initialize nonfastforward in do_push.","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2009-09-16T17:28:27Z","receivedAt":"2009-09-16T17:28:27Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"The variable is assigned unconditionally in print_push_status, but\nprint_push_status is not reached by all codepaths. In particular, this\nfixes a bug where \"git push ... nonexisting-branch\" was complaining about\nnon-fast forward.\n\nSigned-off-by: Matthieu Moy \n---\nNew version initializing nonfastforward inside transport()\n\n transport.c | 1 +\n 1 files changed, 1 insertions(+), 0 deletions(-)\n\ndiff --git a/transport.c b/transport.c\nindex 4cb8077..18db3d3 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -871,6 +871,7 @@ int transport_push(struct transport *transport,\n \t\t int refspec_nr, const char **refspec, int flags,\n \t\t int * nonfastforward)\n {\n+\t*nonfastforward = 0;\n \tverify_remote_names(refspec_nr, refspec);\n \n \tif (transport->push)\n-- \n1.6.5.rc1.11.g2d184.dirty\n"},{"id":"123364","messageId":"7vvdjizrgg.fsf@alter.siamese.dyndns.org","threadId":"20974","inReplyTo":"vpqfxamby33.fsf@bauges.imag.fr","subject":"Re: [PATCH] push: Correctly initialize nonfastforward in do_push.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-09-16T18:15:27Z","receivedAt":"2009-09-16T18:15:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy writes:\n\n> Short answer: trust me, without the patch, you get the non-fast\n> forward (and valgrind tells you about conditional jump on\n> uninitialized value), with, you don't ;-).\n\nI understand valgrind one; I can trace the codepath with eyeballs without\nit, and that is why I said it looks correct to begin with.\n\nMy puzzlement was that the following in the log message did not seem to\nreproduce for me:\n\n ... where \"git push ... nonexisting-branch\" was complaining about\n non-fast forward.\n\nI would be eventually writing an entry in the Release Notes about this\nfix, and I do not want to say:\n\n \"git push $there no-such-ref\" incorrectly said no-such-ref does not\n fast forward; fixed.\n\nwhen I know that command line would produce something entirely different\nerror.\n"},{"id":"123365","messageId":"vpqskemagla.fsf@bauges.imag.fr","threadId":"20974","inReplyTo":"7vvdjizrgg.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] push: Correctly initialize nonfastforward in do_push.","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2009-09-16T18:29:21Z","receivedAt":"2009-09-16T18:29:21Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Junio C Hamano writes:\n\n> Matthieu Moy writes:\n>\n>> Short answer: trust me, without the patch, you get the non-fast\n>> forward (and valgrind tells you about conditional jump on\n>> uninitialized value), with, you don't ;-).\n>\n> I understand valgrind one; I can trace the codepath with eyeballs without\n> it, and that is why I said it looks correct to begin with.\n\n(in case it wasn't clear, \"you\" in my message should have been read as\n\"one\", no harm intended)\n\n> My puzzlement was that the following in the log message did not seem to\n> reproduce for me:\n> ... where \"git push ... nonexisting-branch\" was complaining about\n> non-fast forward.\n\nIt's comming from an uninitialized variable, so it may have worked\njust \"by chance\". For me, it seems to do it reproducibly, with for\nexample:\n\n$ mkdir repo\n$ rm -rf repo/\n$ git init repo\nInitialized empty Git repository in /tmp/repo/.git/\n$ cd repo; touch foo; git add .; git commit -m foo > /dev/null\n$ git push . nonexisting-branch\nerror: src refspec nonexisting-branch does not match any.\nerror: failed to push some refs to '.'\nTo prevent you from losing history, non-fast-forward updates were rejected\nMerge the remote changes before pushing again. See the 'non-fast forward'\nsection of 'git push --help' for details.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"}]}