{"thread":{"id":"20639","subject":"[PATCH 05/11] Remove va_copy at MSVC because there are va_copy.","startedAt":"2009-08-17T16:04:00Z","lastAt":"2009-08-18T09:54:43Z","messageCount":10,"participants":["Frank Li","Johannes Schindelin","Paolo Bonzini","Reece Dunn","Erik Faye-Lund","Joshua Jensen"],"isPatch":true,"patchVersion":1,"patchTotal":11},"messages":[{"id":"120911","messageId":"1250525040-5868-1-git-send-email-lznuaa@gmail.com","threadId":"20639","inReplyTo":null,"subject":"[PATCH 05/11] Remove va_copy at MSVC because there are va_copy.","fromName":"Frank Li","fromEmail":"lznuaa@gmail.com","sentAt":"2009-08-17T16:04:00Z","receivedAt":"2009-08-17T16:04:00Z","isPatch":true,"sender":{"key":"lznuaa@gmail.com","avatar":"https://avatars.githubusercontent.com/u/40642?v=4"},"body":"MSVs have not implemented va_copy. remove va_copy at MSVC environment.\nIt will malloc buffer each time.\n\nSigned-off-by: Frank Li \n---\n compat/winansi.c | 8 ++++++++\n 1 files changed, 8 insertions(+), 0 deletions(-)\n\ndiff --git a/compat/winansi.c b/compat/winansi.c\nindex 9217c24..6091138 100644\n--- a/compat/winansi.c\n+++ b/compat/winansi.c\n@@ -3,7 +3,11 @@\n */\n \n #include \n+#ifdef _MSC_VER\n+#include \n+#else\n #include \"../git-compat-util.h\"\n+#endif\n \n /*\n Functions to be wrapped:\n@@ -310,9 +314,13 @@ static int winansi_vfprintf(FILE *stream, const char *format, va_list list)\n \tif (!console)\n \t\tgoto abort;\n \n+#ifndef _MSC_VER \n \tva_copy(cp, list);\n \tlen = vsnprintf(small_buf, sizeof(small_buf), format, cp);\n \tva_end(cp);\n+#else\n+\tlen= sizeof(small_buf) ;\n+#endif\n \n \tif (len > sizeof(small_buf) - 1) {\n \t\tbuf = malloc(len + 1);\n-- \n1.6.4.msysgit.0\n"},{"id":"120933","messageId":"alpine.DEB.1.00.0908171840450.4991@intel-tinevez-2-302","threadId":"20639","inReplyTo":"1250525040-5868-1-git-send-email-lznuaa@gmail.com","subject":"Re: [PATCH 05/11] Remove va_copy at MSVC because there are va_copy.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-08-17T16:49:41Z","receivedAt":"2009-08-17T16:49:41Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 18 Aug 2009, Frank Li wrote:\n\n> MSVs have not implemented va_copy. remove va_copy at MSVC environment.\n> It will malloc buffer each time.\n> \n> Signed-off-by: Frank Li \n\nHow about this instead?\n\n\tWork around Microsoft Visual C++ not having va_copy()\n\n\tIn winansi.c, Git wants to know the length of the formatted string \n\tso it can allocate enough space for it. But Microsoft Visual C++\n\tdoes not have va_copy(), so we have to guess.\n\nThe problem is the guessing part:\n\n> diff --git a/compat/winansi.c b/compat/winansi.c\n> index 9217c24..6091138 100644\n> --- a/compat/winansi.c\n> +++ b/compat/winansi.c\n> @@ -310,9 +314,13 @@ static int winansi_vfprintf(FILE *stream, const char *format, va_list list)\n> \tif (!console)\n> \t\tgoto abort;\n> \n> +#ifndef _MSC_VER \n> \tva_copy(cp, list);\n> \tlen = vsnprintf(small_buf, sizeof(small_buf), format, cp);\n> \tva_end(cp);\n> +#else\n> +\tlen= sizeof(small_buf) ;\n> +#endif\n\nsmall_buf only is 256 bytes. How do you want to make sure that the \nsubsequent vsnprintf() is not writing outside of the buffer?\n\nAlso, you still miss a space between \"len\" and \"=\".\n\nCiao,\nDscho\n"},{"id":"120935","messageId":"4A898B27.3040507@gnu.org","threadId":"20639","inReplyTo":"1250525040-5868-1-git-send-email-lznuaa@gmail.com","subject":"Re: [PATCH 05/11] Remove va_copy at MSVC because there are va_copy.","fromName":"Paolo Bonzini","fromEmail":"bonzini@gnu.org","sentAt":"2009-08-17T16:53:59Z","receivedAt":"2009-08-17T16:53:59Z","isPatch":true,"sender":{"key":"bonzini@gnu.org","avatar":"https://avatars.githubusercontent.com/u/42082?v=4"},"body":"On 08/17/2009 06:04 PM, Frank Li wrote:\n> MSVs have not implemented va_copy. remove va_copy at MSVC environment.\n> It will malloc buffer each time.\n\n... but only a 257-byte buffer as dscho pointed out.\n\nIn many places that do not have va_copy, a simple assignment works. And \nva_end is almost always a no-op. So what about\n\n#ifndef va_copy\n#define va_copy(dst, src)\t((dst) = (src))\n#endif\n\nif it works on MSVC?\n\nPaolo\n"},{"id":"120936","messageId":"3f4fd2640908170956j556fcba7o1ec4c2107197dbd@mail.gmail.com","threadId":"20639","inReplyTo":"4A898B27.3040507@gnu.org","subject":"Re: [PATCH 05/11] Remove va_copy at MSVC because there are va_copy.","fromName":"Reece Dunn","fromEmail":"msclrhd@googlemail.com","sentAt":"2009-08-17T16:56:17Z","receivedAt":"2009-08-17T16:56:17Z","isPatch":true,"sender":{"key":"msclrhd@googlemail.com","avatar":null},"body":"2009/8/17 Paolo Bonzini :\n> On 08/17/2009 06:04 PM, Frank Li wrote:\n>>\n>> MSVs have not implemented va_copy. remove va_copy at MSVC environment.\n>> It will malloc buffer each time.\n>\n> ... but only a 257-byte buffer as dscho pointed out.\n>\n> In many places that do not have va_copy, a simple assignment works.  And\n> va_end is almost always a no-op. So what about\n>\n> #ifndef va_copy\n> #define va_copy(dst, src)       ((dst) = (src))\n> #endif\n>\n> if it works on MSVC?\n\nAccording to http://stackoverflow.com/questions/558223/vacopy-porting-to-visual-c\nthat should work.\n\n- Reece\n"},{"id":"120939","messageId":"40aa078e0908171002j4b610fe4j34a4e7d3081a9efa@mail.gmail.com","threadId":"20639","inReplyTo":"4A898B27.3040507@gnu.org","subject":"Re: [PATCH 05/11] Remove va_copy at MSVC because there are va_copy.","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@googlemail.com","sentAt":"2009-08-17T17:02:18Z","receivedAt":"2009-08-17T17:02:18Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Mon, Aug 17, 2009 at 6:53 PM, Paolo Bonzini wrote:\n> #ifndef va_copy\n> #define va_copy(dst, src)       ((dst) = (src))\n> #endif\n\nAre you sure va_copy is always a preprocessor symbol? How about\n\n#ifdef _MSC_VER\n#define va_copy(dst, src) ((dst) = (src))\n#endif\n\ninstead? It'd make me sleep slightly better at night, at least ;)\n\n-- \nErik \"kusma\" Faye-Lund\nkusmabite@gmail.com\n(+47) 986 59 656\n"},{"id":"120947","messageId":"40aa078e0908171026h4a92d249u4e2b560e01696303@mail.gmail.com","threadId":"20639","inReplyTo":"40aa078e0908171002j4b610fe4j34a4e7d3081a9efa@mail.gmail.com","subject":"Re: [PATCH 05/11] Remove va_copy at MSVC because there are va_copy.","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@googlemail.com","sentAt":"2009-08-17T17:26:19Z","receivedAt":"2009-08-17T17:26:19Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Mon, Aug 17, 2009 at 7:02 PM, Erik Faye-Lund wrote:\n> Are you sure va_copy is always a preprocessor symbol?\n\nAccording to the following forum-post we are:\nhttp://www.velocityreviews.com/forums/showpost.php?p=1689162&postcount=2\n\nHowever, I decided to dig a bit further, so I had a look at the public\ndraft spec at http://www.open-std.org/JTC1/SC22/WG14/www/docs/n1256.pdf,\nsection 7.15.1:\n\n\"The va_start and va_arg macros described in this subclause shall be implemented\nas macros, not functions. It is unspecified whether va_copy and va_end\nare macros or\nidentifiers declared with external linkage.\"\n\nI don't have access (that I know of) to the finalized spec, but it\nlooks sketchy to me to depend on va_copy being implemented as a macro\ngiven this wording.\n\n-- \nErik \"kusma\" Faye-Lund\nkusmabite@gmail.com\n(+47) 986 59 656\n"},{"id":"120955","messageId":"4A899FFD.3080607@workspacewhiz.com","threadId":"20639","inReplyTo":"alpine.DEB.1.00.0908171840450.4991@intel-tinevez-2-302","subject":"Re: [PATCH 05/11] Remove va_copy at MSVC because there are va_copy.","fromName":"Joshua Jensen","fromEmail":"jjensen@workspacewhiz.com","sentAt":"2009-08-17T18:22:53Z","receivedAt":"2009-08-17T18:22:53Z","isPatch":true,"sender":{"key":"jjensen@workspacewhiz.com","avatar":"https://avatars.githubusercontent.com/u/111687?v=4"},"body":"----- Original Message -----\nFrom: Johannes Schindelin\nDate: 8/17/2009 10:49 AM\n> How about this instead?\n>\n> \tWork around Microsoft Visual C++ not having va_copy()\n>\n> \tIn winansi.c, Git wants to know the length of the formatted string \n> \tso it can allocate enough space for it. But Microsoft Visual C++\n> \tdoes not have va_copy(), so we have to guess\nI did not look at the surrounding code, but could Microsoft's C runtime \nextension _vscprintf, which returns the number of characters in the \nformatted string, be of use here?\n\nJosh\n"},{"id":"120967","messageId":"alpine.DEB.1.00.0908172144460.8306@pacific.mpi-cbg.de","threadId":"20639","inReplyTo":"40aa078e0908171002j4b610fe4j34a4e7d3081a9efa@mail.gmail.com","subject":"Re: [PATCH 05/11] Remove va_copy at MSVC because there are va_copy.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-08-17T19:46:14Z","receivedAt":"2009-08-17T19:46:14Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 17 Aug 2009, Erik Faye-Lund wrote:\n\n> On Mon, Aug 17, 2009 at 6:53 PM, Paolo Bonzini wrote:\n> > #ifndef va_copy\n> > #define va_copy(dst, src)       ((dst) = (src))\n> > #endif\n> \n> Are you sure va_copy is always a preprocessor symbol? How about\n> \n> #ifdef _MSC_VER\n> #define va_copy(dst, src) ((dst) = (src))\n> #endif\n\nWhy not #define it in compat/msvc.h? Or introduce a \nDEFINE_VA_COPY_TRIVIALLY symbol or some such?\n\nCiao,\nDscho"},{"id":"121052","messageId":"1976ea660908172206hc75c1e6i117806338be5ccea@mail.gmail.com","threadId":"20639","inReplyTo":"4A898B27.3040507@gnu.org","subject":"Re: [PATCH 05/11] Remove va_copy at MSVC because there are va_copy.","fromName":"Frank Li","fromEmail":"lznuaa@gmail.com","sentAt":"2009-08-18T05:06:23Z","receivedAt":"2009-08-18T05:06:23Z","isPatch":true,"sender":{"key":"lznuaa@gmail.com","avatar":"https://avatars.githubusercontent.com/u/40642?v=4"},"body":">\n> #ifndef va_copy\n> #define va_copy(dst, src)\t((dst) = (src))\n> #endif\n>\n> if it works on MSVC?\n>\n> Paolo\n>\n\nI test it, it works.\n"},{"id":"121074","messageId":"alpine.DEB.1.00.0908181153000.4680@intel-tinevez-2-302","threadId":"20639","inReplyTo":"1976ea660908172206hc75c1e6i117806338be5ccea@mail.gmail.com","subject":"Re: [PATCH 05/11] Remove va_copy at MSVC because there are va_copy.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-08-18T09:54:43Z","receivedAt":"2009-08-18T09:54:43Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nHi,\n\nOn Tue, 18 Aug 2009, Frank Li wrote:\n\n> > #ifndef va_copy\n> > #define va_copy(dst, src)\t((dst) = (src))\n> > #endif\n> >\n> > if it works on MSVC?\n> \n> I test it, it works.\n\nBut please, either put it into compat/msvc.h or make it dependent on some \n#define such as \"DEFINE_VA_COPY_TRIVIALLY\" so that other platforms who \nmight miss va_copy (but can use the trivial definition above) can use it. \nI do not think that va_copy can be defined like this in general.\n\nCiao,\nDscho\n"}]}