{"thread":{"id":"31251","subject":"RE: git on HP NonStop","startedAt":"2012-08-14T15:52:50Z","lastAt":"2012-08-23T09:23:02Z","messageCount":11,"participants":["Joachim Schmitz","Jan Engelhardt","Johannes Sixt","Junio C Hamano","Andreas Ericsson"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"196977","messageId":"004b01cd7a34$dbf2c160$93d84420$@schmitz-digital.de","threadId":"31251","inReplyTo":"7vr4r98rfd.fsf@alter.siamese.dyndns.org","subject":"RE: git on HP NonStop","fromName":"Joachim Schmitz","fromEmail":"jojo@schmitz-digital.de","sentAt":"2012-08-14T15:52:50Z","receivedAt":"2012-08-14T15:52:50Z","isPatch":false,"sender":{"key":"jojo@schmitz-digital.de","avatar":"https://avatars.githubusercontent.com/u/1786669?v=4"},"body":"> From: Junio C Hamano [mailto:gitster@pobox.com]\n> Sent: Tuesday, August 14, 2012 4:44 PM\n> To: Joachim Schmitz\n> Subject: Re: git on HP NonStop\n> \n> \"Joachim Schmitz\" <jojo@schmitz-digital.de> writes:\n> \n> > Interesting, I never mentioned Tandem did I, But still you recognized\n> > HP NonStop as that.\n> \n> No, *you* did in your patch \"#ifdef\".\n\nAh, I see.\n\n> > Well, I do care about that platform,  but if you don't, there's not\n> > much point in me trying to get Tandem specific patches applied, is it?\n> \n> As long as the change is isolated (i.e. compilation without \"#define\nTANDEM\"\n\n__TANDEM actually\n\n> for other people will produce byte-for-byte identical result as before),\nand\n> cleanly made (i.e. the resulting source code is not littered with \"#ifdef\n> TANDEM\" in many places), I do not think there is a reason not to have such\na\n> patchset.\n\nIt isn't in many places, only 2 places in git-compat-util.h so far:\n/usr/local/bin/diff -EBbu ./git-compat-util.h.orig ./git-compat-util.h\n--- ./git-compat-util.h.orig    2012-07-30 15:50:38 -0500\n+++ ./git-compat-util.h 2012-08-12 11:26:46 -0500\n@@ -74,7 +74,8 @@\n # define _XOPEN_SOURCE 500\n # endif\n #elif !defined(__APPLE__) && !defined(__FreeBSD__) && !defined(__USLC__) &&\n\\\n-      !defined(_M_UNIX) && !defined(__sgi) && !defined(__DragonFly__)\n+      !defined(_M_UNIX) && !defined(__sgi) && !defined(__DragonFly__) && \\\n+      !defined(__TANDEM)\n #define _XOPEN_SOURCE 600 /* glibc2 and AIX 5.3L need 500, OpenBSD needs\n600 for S_ISLNK() */\n #define _XOPEN_SOURCE_EXTENDED 1 /* AIX 5.3L needs this */\n #endif\n @@ -98,6 +99,11 @@\n #include <stdlib.h>\n #include <stdarg.h>\n #include <string.h>\n+#ifdef __TANDEM\n+# include <strings.h> /* for strcasecmp() */\n+  typedef int intptr_t; /* not \"int *\" ?!? */\n+  typedef unsigned int uintptr_t; /* not \"unsigned int *\" ?!? */\n+#endif\n #include <errno.h>\n #include <limits.h>\n #include <sys/param.h>\n\nToo much? The 2nd part is not necessary NonStop specific, any idea for a\nbetter way?\n\nThere there's Makefile:\n/usr/local/bin/diff -EBbu ./Makefile.orig ./Makefile\n--- ./Makefile.orig     2012-07-30 15:50:38 -0500\n+++ ./Makefile  2012-08-14 06:07:16 -0500\n@@ -1297,6 +1297,45 @@\n        NO_CURL =\n        NO_EXPAT =\n endif\n+ifeq ($(uname_S),NONSTOP_KERNEL)\n+       CC = cc -c99 # needs some C99 features, \"inline\" is just one of them\n+       CFLAGS= -g -O\n+       prefix = /usr/local\n+\n+       # as detected by './configure'\n+       #NO_CURL = YesPlease # missdetected, disabled, see below\n+       NEEDS_SSL_WITH_CURL = YesPlease # added manually, see above\n+       HAVE_LIBCHARSET_H=YesPlease\n+       NEEDS_LIBICONV = YesPlease # needs libiconv first, changed further\ndown\n+       NO_SYS_SELECT_H=UnfortunatelyYes\n+       NO_D_TYPE_IN_DIRENT = YesPlease\n+       NO_HSTRERROR=YesPlease\n+       NO_STRCASESTR=YesPlease\n+       NO_FNMATCH_CASEFOLD = YesPlease\n+       NO_MEMMEM = YesPlease\n+       NO_STRLCPY = YesPlease\n+       NO_SETENV = YesPlease\n+       NO_UNSETENV = YesPlease\n+       NO_MKDTEMP = YesPlease\n+       NO_MKSTEMPS = YesPlease\n+       OLD_ICONV=UnfortunatelyYes # currently libiconv-1.9.1\n+       NO_REGEX=YesPlease # Why? ToDo?\n+       NO_PTHREADS=UnfortunatelyYes # ToDo? Using PUT, maybe?\n+\n+       # our's are in ${prefix}/bin\n+       PERL_PATH = ${prefix}/bin/perl\n+       PYTHON_PATH = ${prefix}/bin/python\n+\n+       # not detected (nor checked for) by './configure'\n+       COMPAT_CFLAGS += -DSA_RESTART=0 # we don't have SA_RESTART on\nNonStop\n+       COMPAT_CFLAGS += -DHAVE_STRING_H=1 # needed in\ncompat/fnmatch/fnmatch.c\n+       NO_ST_BLOCKS_IN_STRUCT_STAT = YesPlease # we don't have that on\nNonStop\n+       NO_NSEC = YesPlease # we don't have that on NonStop\n+       NO_PREAD = YesPlease # we could use floss_pread though?\n+       NO_MMAP = YesPlease # we could use floss_mmap though?\n+       # newly implemented further down\n+       NO_POLL = YesPlease # we could use floss_poll though?\n+endif\n ifneq (,$(findstring MINGW,$(uname_S)))\n        pathsep = ;\n        NO_PREAD = YesPlease\n@@ -1526,6 +1565,11 @@\n        LIB_4_CRYPTO = $(OPENSSL_LINK) -lcrypto\n endif\n endif\n+ifndef NO_GETTEXT\n+ifndef LIBC_CONTAINS_LIBINTL\n+       EXTLIBS += -lintl\n+endif\n+endif\n ifdef NEEDS_LIBICONV\n        ifdef ICONVDIR\n                BASIC_CFLAGS += -I$(ICONVDIR)/include\n@@ -1538,11 +1582,6 @@\n ifdef NEEDS_LIBGEN\n        EXTLIBS += -lgen\n Endif\n-ifndef NO_GETTEXT\n-ifndef LIBC_CONTAINS_LIBINTL\n-       EXTLIBS += -lintl\n-endif\n-endif\n ifdef NEEDS_SOCKET\n        EXTLIBS += -lsocket\n endif\n@@ -1591,6 +1630,11 @@\n        BASIC_CFLAGS += -DNO_GETTEXT\n        USE_GETTEXT_SCHEME ?= fallthrough\n endif\n+ifdef NO_POLL\n+       NO_SYS_POLL_H = YesPlease\n+       COMPAT_CFLAGS += -DNO_POLL -Icompat/win32 # so it finds poll.h\n+       COMPAT_OBJS += compat/win32/poll.o\n+endif\n ifdef NO_STRCASESTR\n        COMPAT_CFLAGS += -DNO_STRCASESTR\n        COMPAT_OBJS += compat/strcasestr.o\n\nHow das that look to you?\n\n> >> Finally, I would prefer to see any message that is addressed to me as\n> >> the project maintainer to be Cc'ed to the list.  I feel 80% of my\n> >> time writing\n> > this\n> >> message wasted, as the contents of it would have helped not just you\n> >> but 4 other people who want to get their toes in the development\n> >> community, but you robbed them the chance of seeing it.\n> >\n> > Sorry for having wasted your time, but you could just as easily have\n> > CCed the list on your reply, couldn't you?\n> \n> No, responding to a private message in public without permission goes\nagainst\n> generally accepted netiquette (I am OK if you respond to this or to the\nprevious\n> message from me in public).\n\nOK, understood and accepted. Herewith you have my permission ;-)\n\n> > The use of getrlimit(RLIMIT_NOFILE, ...) vs. sysconf(_SC_OPEN_MAX) in\n> > sha1_file.c, I believe the later to be more portable, also it is used\n> > in other places already (namely compat/win32/poll.c)\n> \n> Careful.  Getting rid of the use of getrlimit() from shared code by\nintroducing\n> our own abstraction to be implemented in compat/* for each platform is\nfine,\n> but that does not mean sysconf() is acceptable by all platforms (your\nfinding\n> use of sysconf() in compat/win32 merely means Win is OK with it).\n\nWell. NonStop is OK with it too. \nAnd I've brackets with #ifdef _SC_OPEN_MAX, just like in compat/win23:\n/usr/local/bin/diff -EBbu ./sha1_file.c.orig ./sha1_file.c\n--- ./sha1_file.c.orig  2012-07-30 15:50:38 -0500\n+++ ./sha1_file.c       2012-08-14 02:42:03 -0500\n@@ -747,6 +747,9 @@\n                return error(\"packfile %s index unavailable\", p->pack_name);\n\n        if (!pack_max_fds) {\n+#ifdef _SC_OPEN_MAX\n+               unsigned int max_fds = sysconf(_SC_OPEN_MAX);\n+#else\n                struct rlimit lim;\n                unsigned int max_fds;\n\n@@ -754,6 +757,7 @@\n                        die_errno(\"cannot get RLIMIT_NOFILE\");\n\n                max_fds = lim.rlim_cur;\n+#endif\n\n                /* Save 3 for stdin/stdout/stderr, 22 for work */\n                if (25 < max_fds)\n \nCan it get any better?\n\n> > The use of curl_easy_strerror() in help.c without checking for a\n> > proper curl version (>=7.12.0)\n> \n> I do not recall the minimum version of curl we support offhand, but if\nthere is a\n> clean workaround that would be good.\n\nWell, I really have none but to disable that part:\n/usr/local/bin/diff -EBbu ./http.c.orig ./http.c\n--- ./http.c.orig       2012-07-30 15:50:38 -0500\n+++ ./http.c    2012-08-14 02:49:43 -0500\n@@ -805,10 +805,12 @@\n                                ret = HTTP_REAUTH;\n                        }\n                } else {\n+#if LIBCURL_VERSION_NUM >= 0x070c00 /* no curl_easy_strerror() before that\n*/\n                        if (!curl_errorstr[0])\n                                strlcpy(curl_errorstr,\n \ncurl_easy_strerror(results.curl_result),\n                                        sizeof(curl_errorstr));\n+#endif\n                        ret = HTTP_ERROR;\n                }\n        } else {\n\n\nIt certainly is a workaround, whether it is clean or clean enough for now is\nto you to decide ;-)\n\nBye, Jojo\n"},{"id":"197286","messageId":"alpine.LNX.2.01.1208191824100.29175@frira.zrqbmnf.qr","threadId":"31251","inReplyTo":"004b01cd7a34$dbf2c160$93d84420$@schmitz-digital.de","subject":"RE: git on HP NonStop","fromName":"Jan Engelhardt","fromEmail":"jengelh@inai.de","sentAt":"2012-08-19T16:25:38Z","receivedAt":"2012-08-19T16:25:38Z","isPatch":false,"sender":{"key":"jengelh@inai.de","avatar":"https://avatars.githubusercontent.com/u/8861948?v=4"},"body":"\nOn Tuesday 2012-08-14 17:52, Joachim Schmitz wrote:\n> @@ -98,6 +99,11 @@\n> #include <stdlib.h>\n> #include <stdarg.h>\n> #include <string.h>\n>+#ifdef __TANDEM\n>+# include <strings.h> /* for strcasecmp() */\n>+  typedef int intptr_t; /* not \"int *\" ?!? */\n>+  typedef unsigned int uintptr_t; /* not \"unsigned int *\" ?!? */\n\nOf course not. intptr_t is an integral value capable of holding\na pointer; it is not a pointer to int (because that would really\nbe redundant to int*.)\n"},{"id":"197332","messageId":"000701cd7ebf$af552aa0$0dff7fe0$@schmitz-digital.de","threadId":"31251","inReplyTo":"alpine.LNX.2.01.1208191824100.29175@frira.zrqbmnf.qr","subject":"RE: git on HP NonStop","fromName":"Joachim Schmitz","fromEmail":"jojo@schmitz-digital.de","sentAt":"2012-08-20T10:36:37Z","receivedAt":"2012-08-20T10:36:37Z","isPatch":false,"sender":{"key":"jojo@schmitz-digital.de","avatar":"https://avatars.githubusercontent.com/u/1786669?v=4"},"body":"> From: Jan Engelhardt [mailto:jengelh@inai.de]\n> Sent: Sunday, August 19, 2012 6:26 PM\n> To: Joachim Schmitz\n> Cc: 'Junio C Hamano'; git@vger.kernel.org\n> Subject: RE: git on HP NonStop\n> \n> \n> On Tuesday 2012-08-14 17:52, Joachim Schmitz wrote:\n> > @@ -98,6 +99,11 @@\n> > #include <stdlib.h>\n> > #include <stdarg.h>\n> > #include <string.h>\n> >+#ifdef __TANDEM\n> >+# include <strings.h> /* for strcasecmp() */\n> >+  typedef int intptr_t; /* not \"int *\" ?!? */\n> >+  typedef unsigned int uintptr_t; /* not \"unsigned int *\" ?!? */\n> \n> Of course not. intptr_t is an integral value capable of holding a pointer;\nit is not\n> a pointer to int (because that would really be redundant to int*.)\n\nOK, thanks for the clarification.\n\nAnother issue I stumbled across:\nThere are numerous places (well, some 10) were something like the following\nis done\n\nint var = var;\nchar *othervar = othervar;\n\nHere this leads to Compiler warnings  'variable \"var\" is used before its\nvalue is set' on NonStop. This self-initialization seems to be a GCC\nextension (?), but even gcc has a -Winit-self option to warn about this.\nShouldn't that better be like the following?\n\nint var = 0;\nchar *othervar = NULL;\n\nWhat is the reason for using that self-init stuff? I don't think it is\nreally portable, is it?\n\nBye, Jojo\n"},{"id":"197333","messageId":"503217F2.5070105@viscovery.net","threadId":"31251","inReplyTo":"000701cd7ebf$af552aa0$0dff7fe0$@schmitz-digital.de","subject":"Re: git on HP NonStop","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2012-08-20T10:56:50Z","receivedAt":"2012-08-20T10:56:50Z","isPatch":false,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 8/20/2012 12:36, schrieb Joachim Schmitz:\n> int var = var;\n> char *othervar = othervar;\n> \n> ... \n>\n> What is the reason for using that self-init stuff? I don't think it is\n> really portable, is it?\n\nIt is used to avoid \"var may be used uninitialized\" warnings for some\ncompilers. Officially (according to the C standard), it is undefined\nbehavior. But I've observed considerable resistance by Junio to fix this\nproperly. Therefore, unless you can show that your compiler generates\nunusable code you better live with the \"self-initialization\" warnings.\n\n-- Hannes\n"},{"id":"197334","messageId":"001101cd7ec6$c038ba60$40aa2f20$@schmitz-digital.de","threadId":"31251","inReplyTo":"503217F2.5070105@viscovery.net","subject":"RE: git on HP NonStop","fromName":"Joachim Schmitz","fromEmail":"jojo@schmitz-digital.de","sentAt":"2012-08-20T11:27:14Z","receivedAt":"2012-08-20T11:27:14Z","isPatch":false,"sender":{"key":"jojo@schmitz-digital.de","avatar":"https://avatars.githubusercontent.com/u/1786669?v=4"},"body":"> From: Johannes Sixt [mailto:j.sixt@viscovery.net]\n> Sent: Monday, August 20, 2012 12:57 PM\n> To: Joachim Schmitz\n> Cc: 'Jan Engelhardt'; 'Junio C Hamano'; git@vger.kernel.org\n> Subject: Re: git on HP NonStop\n> \n> Am 8/20/2012 12:36, schrieb Joachim Schmitz:\n> > int var = var;\n> > char *othervar = othervar;\n> >\n> > ...\n> >\n> > What is the reason for using that self-init stuff? I don't think it is\n> > really portable, is it?\n> \n> It is used to avoid \"var may be used uninitialized\" warnings for some\ncompilers.\n\nWell, it results in a similar warning on NonStop. \"var is used before it is\nset\" and I think this is equally bad.\nIn either case we don't know what the content of that var is.\n\nE.g. in wt_status.c the variable 'status' is set at only one place, but\nlater it is switched on. If lucky we get to the default case and die.\nSo why not just \nint status = 0;\n\n> Officially (according to the C standard), it is undefined behavior. \n\nYes, I had that suspicion. Not good to rely in this...\n\n> But I've observed considerable resistance by Junio to fix this properly. \n\nWhat's the  reason behind that?\n\n> Therefore, unless\n> you can show that your compiler generates unusable code you better live\nwith\n> the \"self-initialization\" warnings.\n\nSo far I can't, so I guess I'll have to live with the warnings, but don't\nquite like it.\n \n> -- Hannes\n\nBye, Jojo\n"},{"id":"197417","messageId":"7vfw7hle6l.fsf@alter.siamese.dyndns.org","threadId":"31251","inReplyTo":"503217F2.5070105@viscovery.net","subject":"Re: git on HP NonStop","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-20T16:29:38Z","receivedAt":"2012-08-20T16:29:38Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt <j.sixt@viscovery.net> writes:\n\n> Am 8/20/2012 12:36, schrieb Joachim Schmitz:\n>> int var = var;\n>> char *othervar = othervar;\n>> \n>> ... \n>>\n>> What is the reason for using that self-init stuff? I don't think it is\n>> really portable, is it?\n>\n> It is used to avoid \"var may be used uninitialized\" warnings for some\n> compilers. Officially (according to the C standard), it is undefined\n> behavior. But I've observed considerable resistance by Junio to fix this\n> properly.\n\nI had resisted\n\n\t-\tint foo = foo;\n        +\tint foo = 0;\n\nin the past.  If some compiler is not seeing that \"foo\" is never\nused uninitialized, such a compiler will generate an unnecessary\ninitialization, so it is not a _proper_ fix anyway (in fact, I do\nnot think a proper fix exists, short of simplifying the code so that\nless sophisticated compilers can see that \"foo\" is never used\nuninitialized).\n\nSo, no, I never resisted a \"proper\" fix.  I resisted swapping an\nunsatisfactory workaround with another.\n\nBetween the two unsatisfactory workarounds, the latter (explicit\nand unnecessary assignment to an innocuous value) is lessor of two\nevils, so I do not particularly mind it, though.  Indeed, I think\nmore recent history shows that we have such changes.\n"},{"id":"197438","messageId":"001901cd7f15$98c6ab50$ca5401f0$@schmitz-digital.de","threadId":"31251","inReplyTo":"7vfw7hle6l.fsf@alter.siamese.dyndns.org","subject":"RE: git on HP NonStop","fromName":"Joachim Schmitz","fromEmail":"jojo@schmitz-digital.de","sentAt":"2012-08-20T20:51:38Z","receivedAt":"2012-08-20T20:51:38Z","isPatch":false,"sender":{"key":"jojo@schmitz-digital.de","avatar":"https://avatars.githubusercontent.com/u/1786669?v=4"},"body":"> From: Junio C Hamano [mailto:gitster@pobox.com]\n> Sent: Monday, August 20, 2012 6:30 PM\n> To: Johannes Sixt\n> Cc: Joachim Schmitz; 'Jan Engelhardt'; git@vger.kernel.org\n> Subject: Re: git on HP NonStop\n> \n> Johannes Sixt <j.sixt@viscovery.net> writes:\n> \n> > Am 8/20/2012 12:36, schrieb Joachim Schmitz:\n> >> int var = var;\n> >> char *othervar = othervar;\n> >>\n> >> ...\n> >>\n> >> What is the reason for using that self-init stuff? I don't think it\n> >> is really portable, is it?\n> >\n> > It is used to avoid \"var may be used uninitialized\" warnings for some\n> > compilers. Officially (according to the C standard), it is undefined\n> > behavior. But I've observed considerable resistance by Junio to fix\n> > this properly.\n> \n> I had resisted\n> \n> \t-\tint foo = foo;\n>         +\tint foo = 0;\n> \n> in the past.  If some compiler is not seeing that \"foo\" is never used\nuninitialized,\n> such a compiler will generate an unnecessary initialization, so it is not\na\n> _proper_ fix anyway (in fact, I do not think a proper fix exists, short of\n> simplifying the code so that less sophisticated compilers can see that\n\"foo\" is\n> never used uninitialized).\n> \n> So, no, I never resisted a \"proper\" fix.  I resisted swapping an\nunsatisfactory\n> workaround with another.\n> \n> Between the two unsatisfactory workarounds, the latter (explicit and\n> unnecessary assignment to an innocuous value) is lessor of two evils, so I\ndo not\n> particularly mind it, though.  Indeed, I think more recent history shows\nthat we\n> have such changes.\n\nOK, so let's have a look at code, current git,\nbuiltin/cat-file.c, line 196:\n        void *contents = contents;\n\nThis variable is set later in an if branch (if (print_contents == BATCH),\nbut not in the else branch. It is later used always under the same condition\nas the one under which it is set.\nApparently is is malloc_d storage (there a \"free(content);\"), so there's no\nharm al all in initializing it with NULL, even if it only appeases a stupid\ncompiler.\n\nThe next one, builtin/fast-export.c, line 486:\n                struct commit *commit = commit;\nit is set in a switch statement, but not in every case, as far as I can see.\nHmm, maybe it is, and I just get lost in the code\nAnd it is used directly after the switch, hopefully set to something\nreasonable.\nWhy take the risk and not set it to NULL?\n\nNext one, builtin/rev-list.c, line 390:\n                int reaches = reaches, all = all;\n\n                revs.commits = find_bisection(revs.commits, &reaches, &all,\n                                              bisect_find_all);\n\nSeem pretty pointless to initialize them, provided find_bisection doesn't\nread them. Does it?\nI'm too Next one, lazy to check... I'd just set them to 0 and stop worrying.\n\nNext one, fast-import.c, line 2268:\n        struct object_entry *oe = oe;\n\nos gets set in en if and an else branch, but not in then intermediate else\nif branch!\nIt is checked for !NULL later, so it should really get initialized to NULL\nin the first place!\n\nSame file, line 2437, same variable name, same story!\nSame file, line 2616, variable e, it is used in an if branch but set after\nthat! \nSame file again, line 2917, variable oe again. Same story as above.\n\nNext file, ll-merge.c, line\n        static const struct ll_merge_options default_opts;\nSomewhat different story here, compiler warning claims \" const variable\n\"default_opts\" requires an initializer\"\nPossible fix:\n        static const struct ll_merge_options default_opts = {0};\n\nnext file, match-trees.c, line 75ff:\n                const unsigned char *elem1 = elem1;\n                const unsigned char *elem2 = elem2;\n                const char *path1 = path1;\n                const char *path2 = path2;\n                unsigned mode1 = mode1;\n                unsigned mode2 = mode2;\nSome get set, some not, depending on code path, but all get used, with\npossibly bogus content.\n\nNext file, merge-recursive.c, line 1903:\n        struct tree *mrtree = mrtree;\npassed on my address to another function, which hopefully knows how to treat\nit. It woult be learer and simpler to just have\n        struct tree *mrtree = NULL;\nwouldn't it?\n\nNext file, run-command.c, line 272:\n        int failed_errno = failed_errno;\nSet deeply nested in some cases. Seems to be used reasonably, but again, why\ntake chanses= 0 is a goot errno ;-)\n\nNext file, submodule.c, line 265:\n        struct commit *left = left, *right = right;\nAs far as I can see it is not set properly before it gets used in some\ncases.\n\nNext filen, transport.c, line 106:\n                int cmp = cmp, len;\nI seem to see code paths whet it is used without being set properly\n\nNext file, vcs-svn/svndiff.c, line 299.... oh, that one has been fixed and\ninitialized to -1.\n\nNext (and last) file, wt-status.c, line 267:\n        int status = status;\nIt apparently does get set properly before use. So here it once may once\nagain just make a compiler happy to set it to 0.\n\nBye, Jojo\n"},{"id":"197482","messageId":"7vy5l9f186.fsf@alter.siamese.dyndns.org","threadId":"31251","inReplyTo":"001901cd7f15$98c6ab50$ca5401f0$@schmitz-digital.de","subject":"Re: git on HP NonStop","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-21T02:06:01Z","receivedAt":"2012-08-21T02:06:01Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Joachim Schmitz\" <jojo@schmitz-digital.de> writes:\n\n> OK, so let's have a look at code, current git,\n> builtin/cat-file.c, line 196:\n>         void *contents = contents;\n>\n> This variable is set later in an if branch (if (print_contents == BATCH),\n> but not in the else branch. It is later used always under the same condition\n> as the one under which it is set.\n> Apparently is is malloc_d storage (there a \"free(content);\"), so there's no\n> harm al all in initializing it with NULL, even if it only appeases a stupid\n> compiler.\n\nIt actually is harmful.  See below.\n\n> The next one, builtin/fast-export.c, line 486:\n>                 struct commit *commit = commit;\n> it is set in a switch statement, but not in every case, as far as I can see.\n> Hmm, maybe it is, and I just get lost in the code\n> And it is used directly after the switch, hopefully set to something\n> reasonable.\n> Why take the risk and not set it to NULL?\n\nDitto.\n\n> Next one, builtin/rev-list.c, line 390:\n>                 int reaches = reaches, all = all;\n>\n>                 revs.commits = find_bisection(revs.commits, &reaches, &all,\n>                                               bisect_find_all);\n>\n> Seem pretty pointless to initialize them, provided find_bisection doesn't\n> read them. Does it?\n\nThat is why they are not initializations but marks to the compiler\nto signal \"you may be stupid enough to think they are used before\ninitialized or assigned, but that is not the case\".  Initializing\nthem would be pointless.\n\n> Next one, fast-import.c, line 2268:\n>         struct object_entry *oe = oe;\n>\n> os gets set in en if and an else branch, but not in then intermediate else\n> if branch!\n\nLook again.  If the recent code is too complex for you to\nunderstand, go back to 10e8d68 (Correct compiler warnings in\nfast-import., 2007-02-06) and read the function.\n\nThe control flow of the early part of that function dictates that\neither oe is assigned *or* inline_data is set to 1.  When inline_data\nis false, oe is always set.\n\nThe compiler was too stupid to read that, and that is why the\n(confusing) idiom to mark it for the stupid compiler was used.\n\nThere are a few reasons why I do not think this self-assignment\nidiom or initializing the variable to an innocuous-looking random\nvalue is a particularly good thing to do when you see compiler\nwarnings.\n\nIf the compiler suspects the variable might be unused, you should\nalways look at it and follow the flow yourself.  Once you know it is\na false alarm, you can use the idiom to squelch the warning, and it\nat the same serves as a note to others that you verified the flow\nand made sure it is a false warning.\n\nWhen the next person wants to touch the code, if the person knows\nthe use of the idiom, it only serves as a warning to be extra\ncareful not to introduce a new codepath that reads the variable\nwithout setting, as the compiler no longer helps him.  If the person\nwho touches the code is as clueless as the compiler and cannot\nfollow the codepath to see the variable is never used uninitialized,\nthe result will be a lot worse.\n\nThat is the reason why I do not think the idiom to squelch the\ncompiler is such a good thing.  Careless people touch the code, so\n\"oe = oe\" initialization carefully placed in the original version\ndoes not necessarily stay as a useful documentation.\n\nBut if you use \"oe = NULL\" as a way to squelch the warning in the\nfirst place, it is no better than \"oe = oe\".  In a sense, it is even\nworse, as it just looks like any other initialization and gives a\nfalse impression that the remainder of the code is written in such a\nway that it tolerates oe being NULL in any codepath, or there is\nsome assignment before that before the code reaches places where oe\ncannot be NULL.  That is different from what \"oe = oe\" initializaion\ndocuments---in the codepath protected by \"if (inline_data)\", it\nisn't just \"oe can safely be NULL there\"; instead it is \"oe can\nsafely be *any* value there, because we don't use it\".  Of course,\nif you explicitly initialized oe to NULL, even if you introduce a\ncodepath where oe cannot be NULL later, you won't get a warning from\nthe compiler, so it is no better than \"oe = oe\".\n\nAnd that is the reason why I do not think initialization to an\ninnocuous-looking random value (e.g. NULL) is a good answer, either.\n\nWhen both are not good, replacing \"oe = oe\" with \"oe = NULL\" didn't\nmake much sense, especially when the former _could_ be used better\nby more careful people.  And that is the resistance J6t remembers.\n\nBut when recent compilers started to warn \"oe = oe\" that itself is\nundefined, the equation changed.  The idiom ceased to be a way to\nsquelch the incorrect compiler warning (which was the primary point\nof its use---the documentation value is secondary, as what we\ndocument is \"we are squelching a false alarm\", but we no longer are\nsquelching anything).\n\nSee 4a5de8d (vcs-svn: avoid self-assignment in dummy initialization\nof pre_off, 2012-06-01), and 58ebd98 (vcs-svn/svndiff.c: squelch\nfalse \"unused\" warning from gcc, 2012-01-27) that it updated, for an\nexample of how we are migrating away from \"oe = oe\" idiom.\n\nIn any case, I already said initializing to 0 is OK in my previous\nmessage, I think, so if you are seeing warning from the compiler on\nuninitialized variables in your new code, you know you should\n\n (1) first follow the codepath to make sure it is a false alarm (if\n     it isn't, fix the code);\n\n (2) then see if you can simplify your code so that it is crystal\n     clear even to the stupid compiler that you are not using an\n     uninitialized variable; and\n\n (3) if you can't, then initialize it to some known bad value,\n     avoiding the \"oe = oe\" idiom.\n\nIs there anything more to discuss on this topic?\n"},{"id":"197592","messageId":"002b01cd8084$8459abc0$8d0d0340$@schmitz-digital.de","threadId":"31251","inReplyTo":"7vy5l9f186.fsf@alter.siamese.dyndns.org","subject":"RE: git on HP NonStop","fromName":"Joachim Schmitz","fromEmail":"jojo@schmitz-digital.de","sentAt":"2012-08-22T16:38:09Z","receivedAt":"2012-08-22T16:38:09Z","isPatch":false,"sender":{"key":"jojo@schmitz-digital.de","avatar":"https://avatars.githubusercontent.com/u/1786669?v=4"},"body":"\n\n> -----Original Message-----\n> From: Junio C Hamano [mailto:gitster@pobox.com]\n> Sent: Tuesday, August 21, 2012 4:06 AM\n> To: Joachim Schmitz\n> Cc: 'Johannes Sixt'; 'Jan Engelhardt'; git@vger.kernel.org\n> Subject: Re: git on HP NonStop\n> \n> \"Joachim Schmitz\" <jojo@schmitz-digital.de> writes:\n> \n> > OK, so let's have a look at code, current git, builtin/cat-file.c,\n> > line 196:\n> >         void *contents = contents;\n> >\n> > This variable is set later in an if branch (if (print_contents ==\n> > BATCH), but not in the else branch. It is later used always under the\n> > same condition as the one under which it is set.\n> > Apparently is is malloc_d storage (there a \"free(content);\"), so\n> > there's no harm al all in initializing it with NULL, even if it only\n> > appeases a stupid compiler.\n> \n> It actually is harmful.  See below.\n\nHarmful to initialize with NULL or to use that undefined behavoir?\n\nI checked what our compiler does here: after having warned about \"vlues us\nused before it is set: it actually dies seem to have initializes the value\nto 0 resp. NULL.\nSo here there's no harm done in avoiding undefined behavior and set it to 0\nresp NULL in the first place.\n\n> > The next one, builtin/fast-export.c, line 486:\n> >                 struct commit *commit = commit; it is set in a switch\n> > statement, but not in every case, as far as I can see.\n> > Hmm, maybe it is, and I just get lost in the code And it is used\n> > directly after the switch, hopefully set to something reasonable.\n> > Why take the risk and not set it to NULL?\n> \n> Ditto.\n> \n> > Next one, builtin/rev-list.c, line 390:\n> >                 int reaches = reaches, all = all;\n> >\n> >                 revs.commits = find_bisection(revs.commits, &reaches,\n&all,\n> >                                               bisect_find_all);\n> >\n> > Seem pretty pointless to initialize them, provided find_bisection\n> > doesn't read them. Does it?\n> \n> That is why they are not initializations but marks to the compiler to\nsignal \"you\n> may be stupid enough to think they are used before initialized or\nassigned, but\n> that is not the case\".  Initializing them would be pointless.\n> \n> > Next one, fast-import.c, line 2268:\n> >         struct object_entry *oe = oe;\n> >\n> > os gets set in en if and an else branch, but not in then intermediate\n> > else if branch!\n> \n> Look again.  If the recent code is too complex for you to understand, go\nback to\n> 10e8d68 (Correct compiler warnings in fast-import., 2007-02-06) and read\nthe\n> function.\n> \n> The control flow of the early part of that function dictates that either\noe is\n> assigned *or* inline_data is set to 1.  When inline_data is false, oe is\nalways\n> set.\n> \n> The compiler was too stupid to read that, and that is why the\n> (confusing) idiom to mark it for the stupid compiler was used.\n> \n> There are a few reasons why I do not think this self-assignment idiom or\n> initializing the variable to an innocuous-looking random value is a\nparticularly\n> good thing to do when you see compiler warnings.\n> \n> If the compiler suspects the variable might be unused, you should always\nlook\n> at it and follow the flow yourself.  Once you know it is a false alarm,\nyou can\n> use the idiom to squelch the warning, and it at the same serves as a note\nto\n> others that you verified the flow and made sure it is a false warning.\n> \n> When the next person wants to touch the code, if the person knows the use\nof\n> the idiom, it only serves as a warning to be extra careful not to\nintroduce a new\n> codepath that reads the variable without setting, as the compiler no\nlonger\n> helps him.  If the person who touches the code is as clueless as the\ncompiler\n> and cannot follow the codepath to see the variable is never used\nuninitialized,\n> the result will be a lot worse.\n> \n> That is the reason why I do not think the idiom to squelch the compiler is\nsuch a\n> good thing.  Careless people touch the code, so \"oe = oe\" initialization\ncarefully\n> placed in the original version does not necessarily stay as a useful\n> documentation.\n> \n> But if you use \"oe = NULL\" as a way to squelch the warning in the first\nplace, it\n> is no better than \"oe = oe\".  In a sense, it is even worse, as it just\nlooks like any\n> other initialization and gives a false impression that the remainder of\nthe code\n> is written in such a way that it tolerates oe being NULL in any codepath,\nor\n> there is some assignment before that before the code reaches places where\noe\n> cannot be NULL.  That is different from what \"oe = oe\" initializaion\ndocuments--\n> -in the codepath protected by \"if (inline_data)\", it isn't just \"oe can\nsafely be\n> NULL there\"; instead it is \"oe can safely be *any* value there, because we\ndon't\n> use it\".  Of course, if you explicitly initialized oe to NULL, even if you\nintroduce\n> a codepath where oe cannot be NULL later, you won't get a warning from the\n> compiler, so it is no better than \"oe = oe\".\n> \n> And that is the reason why I do not think initialization to an\ninnocuous-looking\n> random value (e.g. NULL) is a good answer, either.\n> \n> When both are not good, replacing \"oe = oe\" with \"oe = NULL\" didn't make\n> much sense, especially when the former _could_ be used better by more\n> careful people.  And that is the resistance J6t remembers.\n> \n> But when recent compilers started to warn \"oe = oe\" that itself is\nundefined,\n> the equation changed.  The idiom ceased to be a way to squelch the\nincorrect\n> compiler warning (which was the primary point of its use---the\ndocumentation\n> value is secondary, as what we document is \"we are squelching a false\nalarm\",\n> but we no longer are squelching anything).\n> \n> See 4a5de8d (vcs-svn: avoid self-assignment in dummy initialization of\npre_off,\n> 2012-06-01), and 58ebd98 (vcs-svn/svndiff.c: squelch false \"unused\"\nwarning\n> from gcc, 2012-01-27) that it updated, for an example of how we are\nmigrating\n> away from \"oe = oe\" idiom.\n> \n> In any case, I already said initializing to 0 is OK in my previous\nmessage, I think,\n> so if you are seeing warning from the compiler on uninitialized variables\nin\n> your new code, you know you should\n> \n>  (1) first follow the codepath to make sure it is a false alarm (if\n>      it isn't, fix the code);\n> \n>  (2) then see if you can simplify your code so that it is crystal\n>      clear even to the stupid compiler that you are not using an\n>      uninitialized variable; and\n> \n>  (3) if you can't, then initialize it to some known bad value,\n>      avoiding the \"oe = oe\" idiom.\n> \n> Is there anything more to discuss on this topic?\n\nWhich of the ones I told you about should get fixed?\n\nBye, Jojo\n"},{"id":"197699","messageId":"5035E887.3030209@op5.se","threadId":"31251","inReplyTo":"002b01cd8084$8459abc0$8d0d0340$@schmitz-digital.de","subject":"Re: git on HP NonStop","fromName":"Andreas Ericsson","fromEmail":"ae@op5.se","sentAt":"2012-08-23T08:23:35Z","receivedAt":"2012-08-23T08:23:35Z","isPatch":false,"sender":{"key":"ae@op5.se","avatar":"https://gravatar.com/avatar/426e89595c75a8f5252dd0c989e5fabe5bcac616e68557427ad9aef6b0ca342a?d=mp&s=160"},"body":"On 08/22/2012 06:38 PM, Joachim Schmitz wrote:\n> \n> \n>> -----Original Message-----\n>> From: Junio C Hamano [mailto:gitster@pobox.com]\n>> Sent: Tuesday, August 21, 2012 4:06 AM\n>> To: Joachim Schmitz\n>> Cc: 'Johannes Sixt'; 'Jan Engelhardt'; git@vger.kernel.org\n>> Subject: Re: git on HP NonStop\n>>\n>> \"Joachim Schmitz\" <jojo@schmitz-digital.de> writes:\n>>\n>>> OK, so let's have a look at code, current git, builtin/cat-file.c,\n>>> line 196:\n>>>          void *contents = contents;\n>>>\n>>> This variable is set later in an if branch (if (print_contents ==\n>>> BATCH), but not in the else branch. It is later used always under the\n>>> same condition as the one under which it is set.\n>>> Apparently is is malloc_d storage (there a \"free(content);\"), so\n>>> there's no harm al all in initializing it with NULL, even if it only\n>>> appeases a stupid compiler.\n>>\n>> It actually is harmful.  See below.\n> \n> Harmful to initialize with NULL or to use that undefined behavoir?\n> \n> I checked what our compiler does here: after having warned about \"vlues us\n> used before it is set: it actually dies seem to have initializes the value\n> to 0 resp. NULL.\n> So here there's no harm done in avoiding undefined behavior and set it to 0\n> resp NULL in the first place.\n> \n\nThere is harm in tricking future programmers into thinking that the\ninitialization actually means something, which some of them do.\n\nIt's unlikely that you're the one to maintain that code forever, and\nthe \"var = var\" idiom is used widely within git with a clear meaning\nas a hint to programmers who read a bit of git code. If they aren't\nused to that idiom, they usually investigate it in the code and\npretty quickly realize that what it means.\n\n\n-- \nAndreas Ericsson                   andreas.ericsson@op5.se\nOP5 AB                             www.op5.se\nTel: +46 8-230225                  Fax: +46 8-230231\n\nConsidering the successes of the wars on alcohol, poverty, drugs and\nterror, I think we should give some serious thought to declaring war\non peace.\n"},{"id":"197706","messageId":"000001cd8110$e5954fc0$b0bfef40$@schmitz-digital.de","threadId":"31251","inReplyTo":"5035E887.3030209@op5.se","subject":"RE: git on HP NonStop","fromName":"Joachim Schmitz","fromEmail":"jojo@schmitz-digital.de","sentAt":"2012-08-23T09:23:02Z","receivedAt":"2012-08-23T09:23:02Z","isPatch":false,"sender":{"key":"jojo@schmitz-digital.de","avatar":"https://avatars.githubusercontent.com/u/1786669?v=4"},"body":"> From: Andreas Ericsson [mailto:ae@op5.se]\n> Sent: Thursday, August 23, 2012 10:24 AM\n> To: Joachim Schmitz\n> Cc: 'Junio C Hamano'; 'Johannes Sixt'; 'Jan Engelhardt'; git@vger.kernel.org\n> Subject: Re: git on HP NonStop\n> \n> On 08/22/2012 06:38 PM, Joachim Schmitz wrote:\n> >\n> >\n> >> -----Original Message-----\n> >> From: Junio C Hamano [mailto:gitster@pobox.com]\n> >> Sent: Tuesday, August 21, 2012 4:06 AM\n> >> To: Joachim Schmitz\n> >> Cc: 'Johannes Sixt'; 'Jan Engelhardt'; git@vger.kernel.org\n> >> Subject: Re: git on HP NonStop\n> >>\n> >> \"Joachim Schmitz\" <jojo@schmitz-digital.de> writes:\n> >>\n> >>> OK, so let's have a look at code, current git, builtin/cat-file.c,\n> >>> line 196:\n> >>>          void *contents = contents;\n> >>>\n> >>> This variable is set later in an if branch (if (print_contents ==\n> >>> BATCH), but not in the else branch. It is later used always under the\n> >>> same condition as the one under which it is set.\n> >>> Apparently is is malloc_d storage (there a \"free(content);\"), so\n> >>> there's no harm al all in initializing it with NULL, even if it only\n> >>> appeases a stupid compiler.\n> >>\n> >> It actually is harmful.  See below.\n> >\n> > Harmful to initialize with NULL or to use that undefined behavoir?\n> >\n> > I checked what our compiler does here: after having warned about \"vlues us\n> > used before it is set: it actually dies seem to have initializes the value\n> > to 0 resp. NULL.\n> > So here there's no harm done in avoiding undefined behavior and set it to 0\n> > resp NULL in the first place.\n> >\n> \n> There is harm in tricking future programmers into thinking that the\n> initialization actually means something, which some of them do.\n\nHmm, OK, I can agree to that.\n\n> It's unlikely that you're the one to maintain that code forever, \n\nIt is unlike for me to ever have to maintain this code.\nCurrently that's Junio's job and I won't apply for in ;-)\n\n> and the \"var = var\" idiom is used widely within git \n\nThis is overstating it a bit. I went thru the entire code and reported all places I could find in an earlier email.\nI went back and counted: It is used in 11 files, at 15 places, for 21 variables. \nOK, I may have missed  a few more that were in code paths my compiler didn't see, but still some 21+ isn't really much.\n\n>with a clear meaning\n\nOnly if you call undefined behavior a  'clear meaning\"!\n\n> as a hint to programmers who read a bit of git code. If they aren't\n> used to that idiom, they usually investigate it in the code and\n> pretty quickly realize that what it means.\n\nWhether I realize what it means, is irrelevant, my compiler does not and warns about it, and as per the ANSI/ICO C standard it\ninvokes undefined behavior.\n\nIf a proper initialization is meaningless for these cases, don't do them at all, let the stupid compiler complain about it and the\nclever programmer check whether the warning is useful, but don't avoid a compiler warning on one compiler by introducing undefined\nbehavior and provoke a compiler warning on another.\n\nBye, Jojo\n"}]}