{"thread":{"id":"18314","subject":"[PATCH 2/2] make the ST_{C,M}TIME_NSEC macros more function like","startedAt":"2009-03-15T11:38:54Z","lastAt":"2009-03-17T17:38:59Z","messageCount":13,"participants":["Kjetil Barvik","Junio C Hamano","Michael J Gruber","Kris Shannon","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"108044","messageId":"cover.1237115791.git.barvik@broadpark.no","threadId":"18314","inReplyTo":null,"subject":"[PATCH 0/2] git checkout: one bugfix and one cosmetic change","fromName":"Kjetil Barvik","fromEmail":"barvik@broadpark.no","sentAt":"2009-03-15T11:38:54Z","receivedAt":"2009-03-15T11:38:54Z","isPatch":true,"sender":{"key":"barvik@broadpark.no","avatar":null},"body":"Just one small bugfix patch, and one small cosmetic change.\n\nBy the way, I wonder how often the list of 'Primary Authors' and\n'Contributors' on the webpage http://git-scm.com/about is updated.\nShould'nt it be updated when a new release, like v1.6.2, is made?\n\nKjetil Barvik (2):\n checkout bugfix: use stat.mtime instead of stat.ctime in two places\n make the ST_{C,M}TIME_NSEC macros more function like\n"},{"id":"108043","messageId":"58b7f6028d593d66e4e181b60f85c8c8dd860aac.1237115791.git.barvik@broadpark.no","threadId":"18314","inReplyTo":"cover.1237115791.git.barvik@broadpark.no","subject":"[PATCH 1/2] checkout bugfix: use stat.mtime instead of stat.ctime in two places","fromName":"Kjetil Barvik","fromEmail":"barvik@broadpark.no","sentAt":"2009-03-15T11:38:55Z","receivedAt":"2009-03-15T11:38:55Z","isPatch":true,"sender":{"key":"barvik@broadpark.no","avatar":null},"body":"Commit e1afca4fd \"write_index(): update index_state->timestamp after\nflushing to disk\" on 2009-02-23 used stat.ctime to record the\ntimestamp of the index-file. This is wrong, so fix this and use the\ncorrect stat.mtime timestamp instead.\n\nCommit 110c46a909 \"Not all systems use st_[cm]tim field for ns\nresolution file timestamp\" on 2009-03-08, has a similar bug for the\nbuiltin-fetch-pack.c file.\n\nSigned-off-by: Kjetil Barvik \n---\n builtin-fetch-pack.c | 2 +-\n read-cache.c | 4 ++--\n 2 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin-fetch-pack.c b/builtin-fetch-pack.c\nindex 0b1a356..d571253 100644\n--- a/builtin-fetch-pack.c\n+++ b/builtin-fetch-pack.c\n@@ -806,7 +806,7 @@ struct ref *fetch_pack(struct fetch_pack_args *my_args,\n \t\t\t\tdie(\"shallow file was removed during fetch\");\n \t\t} else if (st.st_mtime != mtime.sec\n #ifdef USE_NSEC\n-\t\t\t\t|| ST_CTIME_NSEC(st) != mtime.nsec\n+\t\t\t\t|| ST_MTIME_NSEC(st) != mtime.nsec\n #endif\n \t\t\t )\n \t\t\tdie(\"shallow file was changed during fetch\");\ndiff --git a/read-cache.c b/read-cache.c\nindex 7f74c8d..3f58711 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -1563,8 +1563,8 @@ int write_index(struct index_state *istate, int newfd)\n \n \tif (ce_flush(&c, newfd) || fstat(newfd, &st))\n \t\treturn -1;\n-\tistate->timestamp.sec = (unsigned int)st.st_ctime;\n-\tistate->timestamp.nsec = ST_CTIME_NSEC(st);\n+\tistate->timestamp.sec = (unsigned int)st.st_mtime;\n+\tistate->timestamp.nsec = ST_MTIME_NSEC(st);\n \treturn 0;\n }\n \n-- \n1.6.2.GIT\n"},{"id":"108042","messageId":"0681248ac5c9cedf5f42adeeae89966a89e6d42a.1237115791.git.barvik@broadpark.no","threadId":"18314","inReplyTo":"cover.1237115791.git.barvik@broadpark.no","subject":"[PATCH 2/2] make the ST_{C,M}TIME_NSEC macros more function like","fromName":"Kjetil Barvik","fromEmail":"barvik@broadpark.no","sentAt":"2009-03-15T11:38:56Z","receivedAt":"2009-03-15T11:38:56Z","isPatch":true,"sender":{"key":"barvik@broadpark.no","avatar":null},"body":"Make the macros take a pointer to a 'struct stat'. This is so that it\nshould be easier to understand what is going on, and that the macros\ncan later be implemented as a inline function if we want to.\n\nImpact: cosmetic change\n\nSigned-off-by: Kjetil Barvik \n---\n builtin-fetch-pack.c | 4 ++--\n git-compat-util.h | 8 ++++----\n read-cache.c | 12 ++++++------\n 3 files changed, 12 insertions(+), 12 deletions(-)\n\ndiff --git a/builtin-fetch-pack.c b/builtin-fetch-pack.c\nindex d571253..0cd50f3 100644\n--- a/builtin-fetch-pack.c\n+++ b/builtin-fetch-pack.c\n@@ -800,13 +800,13 @@ struct ref *fetch_pack(struct fetch_pack_args *my_args,\n \t\tint fd;\n \n \t\tmtime.sec = st.st_mtime;\n-\t\tmtime.nsec = ST_MTIME_NSEC(st);\n+\t\tmtime.nsec = ST_MTIME_NSEC(&st);\n \t\tif (stat(shallow, &st)) {\n \t\t\tif (mtime.sec)\n \t\t\t\tdie(\"shallow file was removed during fetch\");\n \t\t} else if (st.st_mtime != mtime.sec\n #ifdef USE_NSEC\n-\t\t\t\t|| ST_MTIME_NSEC(st) != mtime.nsec\n+\t\t\t\t|| ST_MTIME_NSEC(&st) != mtime.nsec\n #endif\n \t\t\t )\n \t\t\tdie(\"shallow file was changed during fetch\");\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 1906253..4a633be 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -394,11 +394,11 @@ void git_qsort(void *base, size_t nmemb, size_t size,\n #define ST_MTIME_NSEC(st) 0\n #else\n #ifdef USE_ST_TIMESPEC\n-#define ST_CTIME_NSEC(st) ((unsigned int)((st).st_ctimespec.tv_nsec))\n-#define ST_MTIME_NSEC(st) ((unsigned int)((st).st_mtimespec.tv_nsec))\n+#define ST_CTIME_NSEC(st) ((unsigned int)((st)->st_ctimespec.tv_nsec))\n+#define ST_MTIME_NSEC(st) ((unsigned int)((st)->st_mtimespec.tv_nsec))\n #else\n-#define ST_CTIME_NSEC(st) ((unsigned int)((st).st_ctim.tv_nsec))\n-#define ST_MTIME_NSEC(st) ((unsigned int)((st).st_mtim.tv_nsec))\n+#define ST_CTIME_NSEC(st) ((unsigned int)((st)->st_ctim.tv_nsec))\n+#define ST_MTIME_NSEC(st) ((unsigned int)((st)->st_mtim.tv_nsec))\n #endif\n #endif\n \ndiff --git a/read-cache.c b/read-cache.c\nindex 3f58711..cff85e3 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -69,8 +69,8 @@ void fill_stat_cache_info(struct cache_entry *ce, struct stat *st)\n {\n \tce->ce_ctime.sec = (unsigned int)st->st_ctime;\n \tce->ce_mtime.sec = (unsigned int)st->st_mtime;\n-\tce->ce_ctime.nsec = ST_CTIME_NSEC(*st);\n-\tce->ce_mtime.nsec = ST_MTIME_NSEC(*st);\n+\tce->ce_ctime.nsec = ST_CTIME_NSEC(st);\n+\tce->ce_mtime.nsec = ST_MTIME_NSEC(st);\n \tce->ce_dev = st->st_dev;\n \tce->ce_ino = st->st_ino;\n \tce->ce_uid = st->st_uid;\n@@ -204,9 +204,9 @@ static int ce_match_stat_basic(struct cache_entry *ce, struct stat *st)\n \t\tchanged |= CTIME_CHANGED;\n \n #ifdef USE_NSEC\n-\tif (ce->ce_mtime.nsec != ST_MTIME_NSEC(*st))\n+\tif (ce->ce_mtime.nsec != ST_MTIME_NSEC(st))\n \t\tchanged |= MTIME_CHANGED;\n-\tif (trust_ctime && ce->ce_ctime.nsec != ST_CTIME_NSEC(*st))\n+\tif (trust_ctime && ce->ce_ctime.nsec != ST_CTIME_NSEC(st))\n \t\tchanged |= CTIME_CHANGED;\n #endif\n \n@@ -1299,7 +1299,7 @@ int read_index_from(struct index_state *istate, const char *path)\n \t\tdst_offset += ce_size(ce);\n \t}\n \tistate->timestamp.sec = st.st_mtime;\n-\tistate->timestamp.nsec = ST_MTIME_NSEC(st);\n+\tistate->timestamp.nsec = ST_MTIME_NSEC(&st);\n \n \twhile (src_offset <= mmap_size - 20 - 8) {\n \t\t/* After an array of active_nr index entries,\n@@ -1564,7 +1564,7 @@ int write_index(struct index_state *istate, int newfd)\n \tif (ce_flush(&c, newfd) || fstat(newfd, &st))\n \t\treturn -1;\n \tistate->timestamp.sec = (unsigned int)st.st_mtime;\n-\tistate->timestamp.nsec = ST_MTIME_NSEC(st);\n+\tistate->timestamp.nsec = ST_MTIME_NSEC(&st);\n \treturn 0;\n }\n \n-- \n1.6.2.GIT\n"},{"id":"108049","messageId":"7vab7m8x5w.fsf@gitster.siamese.dyndns.org","threadId":"18314","inReplyTo":"cover.1237115791.git.barvik@broadpark.no","subject":"Re: [PATCH 0/2] git checkout: one bugfix and one cosmetic change","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-03-15T18:21:15Z","receivedAt":"2009-03-15T18:21:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Kjetil Barvik writes:\n\n> Just one small bugfix patch, and one small cosmetic change.\n>\n> By the way, I wonder how often the list of 'Primary Authors' and\n> 'Contributors' on the webpage http://git-scm.com/about is updated.\n> Should'nt it be updated when a new release, like v1.6.2, is made?\n\nThanks for noticing. Though git-scm.com is not under my control, the site\nis considered the official git homepage these days, and I am glad to see\nimprovements to its contents discussed here. I do not see Scott very\noften on this list these days, so I am CC'ing him.\n"},{"id":"108053","messageId":"7viqma7f00.fsf@gitster.siamese.dyndns.org","threadId":"18314","inReplyTo":"7vab7m8x5w.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 0/2] git checkout: one bugfix and one cosmetic change","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-03-15T19:38:55Z","receivedAt":"2009-03-15T19:38:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Kjetil Barvik writes:\n>\n>> Just one small bugfix patch, and one small cosmetic change.\n>>\n>> By the way, I wonder how often the list of 'Primary Authors' and\n>> 'Contributors' on the webpage http://git-scm.com/about is updated.\n>> Should'nt it be updated when a new release, like v1.6.2, is made?\n>\n> Thanks for noticing. Though git-scm.com is not under my control, the site\n> is considered the official git homepage these days, and I am glad to see\n> improvements to its contents discussed here. I do not see Scott very\n> often on this list these days, so I am CC'ing him.\n\nRagarding the list on the page, I have one thought (not complete enough to\nbe called suggestion) and one datapoint:\n\n (1) The boundary between the \"Primary Authors\" vs \"Contributors\" seems to\n be set at 50 commits with the current table. This would mean that we\n will have a lot more primary authors as project progresses. Is this\n desirable?\n\n We have 14999 non-merge commits as of 1.6.2; perhaps a per-cent (or a\n half per-cent) cutoff rule would give a more balanced and consistent\n view in the longer term [*1*]?\n\n (2) This script:\n\n $ git shortlog -s v1.6.1 | sed -e 's/^[ 0-9]*//' >/var/tmp/1\n $ git shortlog -s v1.6.2 | sed -e 's/^[ 0-9]*//' >/var/tmp/2\n $ comm -13 /var/tmp/[12]\n\n produces the list of new contributors. There are 39 names [*2*].\n\n[Footnotes]\n\n*1* A more drastic change would be not to have two lists, but just one.\n\n*2* Thanks and welcome.\n\n Allan Caffee, Ben Walton, Benjamin Kramer, Benjamin Sergeant, Benoit\n Sigoure, Danijel Tasov, David J. Mellor, Devin Doucette, Dévai Tamás,\n Elijah Newren, Eric Kidd, Fabian Franz, Felipe Contreras, Geoffrey\n Thomas, Henrik Austad, Jacob Helwig, Jake Goulding, Jeremy White,\n Johannes Gilger, Jonas Flodén, Keith Cascio, Kjetil Barvik, Marc\n Branchaud, Marc-Andre Lureau, Nazri Ramliy, Pat Notz, Paul Jarc, Peter\n Oberndorfer, Ray Chuan, Roy Lee, Serge van den Boom, Sergei Organov,\n Stefan Karpinski, Tay Ray Chuan, Ted Pavlic, Tor Arne Vestbø, Vitaly\n \"_Vi\" Shukela, Väinö Järvelä, jidanni@jidanni.org.\n"},{"id":"108055","messageId":"7v4oxu7dyn.fsf@gitster.siamese.dyndns.org","threadId":"18314","inReplyTo":"0681248ac5c9cedf5f42adeeae89966a89e6d42a.1237115791.git.barvik@broadpark.no","subject":"Re: [PATCH 2/2] make the ST_{C,M}TIME_NSEC macros more function like","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-03-15T20:01:20Z","receivedAt":"2009-03-15T20:01:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Kjetil Barvik writes:\n\n> Make the macros take a pointer to a 'struct stat'. This is so that it\n> should be easier to understand what is going on, and that the macros\n> can later be implemented as a inline function if we want to.\n>\n> Impact: cosmetic change\n\nHmm,...\n\nI have to wonder if this cosmetic change is an improvement, though.\n\nI do not have a strong feeling either way, but I think it makes it clear\nthat these two macros are not lvalues if you do not pass a pointer but\ninstead pass a structure. An inline function can still take a structure\npassed by value as an argument anyway, no?\n"},{"id":"108058","messageId":"86tz5u1m7i.fsf@broadpark.no","threadId":"18314","inReplyTo":"7v4oxu7dyn.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 2/2] make the ST_{C,M}TIME_NSEC macros more function like","fromName":"Kjetil Barvik","fromEmail":"barvik@broadpark.no","sentAt":"2009-03-15T21:59:45Z","receivedAt":"2009-03-15T21:59:45Z","isPatch":true,"sender":{"key":"barvik@broadpark.no","avatar":null},"body":"Junio C Hamano writes:\n\n> Kjetil Barvik writes:\n>\n>> Make the macros take a pointer to a 'struct stat'. This is so that it\n>> should be easier to understand what is going on, and that the macros\n>> can later be implemented as a inline function if we want to.\n>>\n>> Impact: cosmetic change\n>\n> Hmm,...\n>\n> I have to wonder if this cosmetic change is an improvement, though.\n>\n> I do not have a strong feeling either way, but I think it makes it\n> clear that these two macros are not lvalues if you do not pass a\n> pointer but instead pass a structure. An inline function can still\n> take a structure passed by value as an argument anyway, no?\n\n It seems to woork from a small gcc test, but since C has call-by-\n value, and http://en.wikipedia.org/wiki/Call_by_value#Call_by_value\n says:\n\n [...] in C or Pascal, calling a function with a large structure as\n an argument will cause the entire structure to be copied,\n potentially causing serious performance degradation, and mutations\n to the structure are invisible to the caller. [...]\n\n So in my eyes it make more sense to be consistent and take the address\n of all struct like objects (&st in this case) for all arguments to\n \"function-like\" things.\n\n But, since these 2 are macros, which use textual substitution, I guess\n things will work correctly either way, and the compiled result will be\n the same. But, I still like the more \"function friendly\" macros.\n\n -- kjetil\n"},{"id":"108070","messageId":"7vhc1ux7nx.fsf@gitster.siamese.dyndns.org","threadId":"18314","inReplyTo":"86tz5u1m7i.fsf@broadpark.no","subject":"Re: [PATCH 2/2] make the ST_{C,M}TIME_NSEC macros more function like","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-03-16T07:12:50Z","receivedAt":"2009-03-16T07:12:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Kjetil Barvik writes:\n\n> [...] in C or Pascal, calling a function with a large structure as\n> an argument will cause the entire structure to be copied,\n> potentially causing serious performance degradation, and mutations\n> to the structure are invisible to the caller. [...]\n>\n> So in my eyes it make more sense to be consistent and take the address\n> of all struct like objects (&st in this case) for all arguments to\n> \"function-like\" things.\n\nNotice the \"mutations to the structure are invisible to the caller\" part.\nThe call site of st_ctime_nsec(st) can be sure that st won't be modified,\nwithout checking the definition of the function.\n\nWhich is actually a nice property. When st_ctime_nsec(st) is implemented as\na macro, you _could_ write it in such a way to mutate what is in st, but\nthe implementation does not do so, and will be unlikely to in the future,\nso I think writing it as if it is a function that receives a structure by\nvalue will help readers of the calling code.\n\nAnd the readability is what we should optimize for when picking from two\nways to write it, and when the generated code is the same.\n"},{"id":"108097","messageId":"49BE7A57.2010901@drmicha.warpmail.net","threadId":"18314","inReplyTo":"cover.1237115791.git.barvik@broadpark.no","subject":"Re: [PATCH 0/2] git checkout: one bugfix and one cosmetic change","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2009-03-16T16:12:07Z","receivedAt":"2009-03-16T16:12:07Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Kjetil Barvik venit, vidit, dixit 15.03.2009 12:38:\n> Just one small bugfix patch, and one small cosmetic change.\n> \n> By the way, I wonder how often the list of 'Primary Authors' and\n> 'Contributors' on the webpage http://git-scm.com/about is updated.\n> Should'nt it be updated when a new release, like v1.6.2, is made?\n\nAssuming it looks at all non-merge commits on master, I can tell you it\nhas been updated after Dec 18 09:55:53 2008 -0800 and before Jan 14\n09:29:24 2009 -0800 ;)\n\nCheers,\nMichael\n"},{"id":"108174","messageId":"e51f4f550903162156i64b64900g815ee8317720f1a0@mail.gmail.com","threadId":"18314","inReplyTo":"cover.1237115791.git.barvik@broadpark.no","subject":"Re: [PATCH 0/2] git checkout: one bugfix and one cosmetic change","fromName":"Kris Shannon","fromEmail":"kris@shannon.id.au","sentAt":"2009-03-17T04:56:12Z","receivedAt":"2009-03-17T04:56:12Z","isPatch":true,"sender":{"key":"kris@shannon.id.au","avatar":"https://gravatar.com/avatar/13a7c0b3c50ffacf54f456e543023fd702898bb134165fa805f019930524151f?d=mp&s=160"},"body":"2009/3/15 Kjetil Barvik :\n> Just one small bugfix patch, and one small cosmetic change.\n>\n> By the way, I wonder how often the list of 'Primary Authors' and\n> 'Contributors' on the webpage http://git-scm.com/about is updated.\n> Should'nt it be updated when a new release, like v1.6.2, is made?\n>\n\nI was rather surprised to see my name on that list. A quick git log\nshowed my one contribution to git-parse-remote way pack in\nAugust 2005.\n\nI'd forgotten about that and was feeling all warm and fuzzy until I did:\ngit log -- git-parse-remote\n\nand saw that it was deleted a week later :(\n"},{"id":"108199","messageId":"20090317084352.GL18475@coredump.intra.peff.net","threadId":"18314","inReplyTo":"e51f4f550903162156i64b64900g815ee8317720f1a0@mail.gmail.com","subject":"Re: [PATCH 0/2] git checkout: one bugfix and one cosmetic change","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-03-17T08:43:52Z","receivedAt":"2009-03-17T08:43:52Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Mar 17, 2009 at 03:56:12PM +1100, Kris Shannon wrote:\n\n> I was rather surprised to see my name on that list. A quick git log\n> showed my one contribution to git-parse-remote way pack in\n> August 2005.\n> \n> I'd forgotten about that and was feeling all warm and fuzzy until I did:\n> git log -- git-parse-remote\n> \n> and saw that it was deleted a week later :(\n\nHeh. The current list just counts commits, which is nice and fast. But\none could also \"git blame\" all of the content from master and credit\npeople based either on:\n\n - number of surviving lines in the current codebase (which obviously\n would give very rankings for people, as the number of lines added\n in a commit is not constant)\n\n - number of commits which have surviving lines\n\nDoing such a calculation would be pretty slow, though, I imagine. And it\nwould of course remove you from the list. :)\n\n-Peff\n"},{"id":"108226","messageId":"49BFA80D.6040504@drmicha.warpmail.net","threadId":"18314","inReplyTo":"20090317084352.GL18475@coredump.intra.peff.net","subject":"Re: [PATCH 0/2] git checkout: one bugfix and one cosmetic change","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2009-03-17T13:39:25Z","receivedAt":"2009-03-17T13:39:25Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Jeff King venit, vidit, dixit 17.03.2009 09:43:\n> On Tue, Mar 17, 2009 at 03:56:12PM +1100, Kris Shannon wrote:\n> \n>> I was rather surprised to see my name on that list. A quick git log\n>> showed my one contribution to git-parse-remote way pack in\n>> August 2005.\n>>\n>> I'd forgotten about that and was feeling all warm and fuzzy until I did:\n>> git log -- git-parse-remote\n>>\n>> and saw that it was deleted a week later :(\n> \n> Heh. The current list just counts commits, which is nice and fast. But\n> one could also \"git blame\" all of the content from master and credit\n> people based either on:\n> \n> - number of surviving lines in the current codebase (which obviously\n> would give very rankings for people, as the number of lines added\n> in a commit is not constant)\n> \n> - number of commits which have surviving lines\n> \n> Doing such a calculation would be pretty slow, though, I imagine. And it\n> would of course remove you from the list. :)\n> \n> -Peff\n\nMaybe we can forge a statement by Canonical, claiming they were among\nthe top contributors to git? Then GKH would do all the statistics for us ;)\n\nMichael\n"},{"id":"108266","messageId":"86bps0t5fw.fsf@broadpark.no","threadId":"18314","inReplyTo":"7vhc1ux7nx.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 2/2] make the ST_{C,M}TIME_NSEC macros more function like","fromName":"Kjetil Barvik","fromEmail":"barvik@broadpark.no","sentAt":"2009-03-17T17:38:59Z","receivedAt":"2009-03-17T17:38:59Z","isPatch":true,"sender":{"key":"barvik@broadpark.no","avatar":null},"body":"Junio C Hamano writes:\n\n> Kjetil Barvik writes:\n>\n>> [...] in C or Pascal, calling a function with a large structure as\n>> an argument will cause the entire structure to be copied,\n>> potentially causing serious performance degradation, and mutations\n>> to the structure are invisible to the caller. [...]\n>>\n>> So in my eyes it make more sense to be consistent and take the address\n>> of all struct like objects (&st in this case) for all arguments to\n>> \"function-like\" things.\n>\n> Notice the \"mutations to the structure are invisible to the caller\" part.\n> The call site of st_ctime_nsec(st) can be sure that st won't be modified,\n> without checking the definition of the function.\n>\n> Which is actually a nice property. When st_ctime_nsec(st) is implemented as\n> a macro, you _could_ write it in such a way to mutate what is in st, but\n> the implementation does not do so, and will be unlikely to in the future,\n> so I think writing it as if it is a function that receives a structure by\n> value will help readers of the calling code.\n>\n> And the readability is what we should optimize for when picking from two\n> ways to write it, and when the generated code is the same.\n\n OK, I guess we can dropp this patch! :-)\n\n -- kjetil\n"}]}