Re: [avrdude-dev] [patch #8790] Add support for Visual Studio Builds

[Wes Witt]

Sorry about the whitespace problems. I've cleaned that all up as well as the 
making the generic fix to the pointer code in flip2 as you suggested.

In answer to your question about the #ifdef change in jtag3.c, the change was 
made because the code inside my #else block makes reference to libusb 
variables, structures, etc. that are not present when HAVE_LIBUSB is not 
defined. Basically the change just prevents some code from being compiled when 
there is no LIBUSB support.

Finally, I need some clarification about the change to stk500. I understand the 
desire for uniformity here -- great. I'm not sure what you're suggesting the 
change be?  On Windows we need tp use alloca in the case because the code is 
declaring a stack variable whose size is determined at runtime. The VS compiler 
cannot do this. If we want the same code on Windows & Linux don't we need to 
use alloca in both cases?

Thanks,
Wes

-----Original Message-----
From: Joerg Wunsch [mailto:address@hidden 
Sent: Monday, November 16, 2015 12:04 AM
To: Joerg Wunsch <address@hidden>; Wes Witt <address@hidden>; address@hidden
Subject: [patch #8790] Add support for Visual Studio Builds

Update of patch #8790 (project avrdude):

                  Status:                    None => In Progress            
             Assigned to:                    None => joerg_wunsch           

    _______________________________________________________

Follow-up Comment #1:

The diff looks mostly good, and makes sense.

In some occasions, there are white-space only changes (different indentation, 
blank lines added or removed etc.), which must never be submitted along with 
actual code changes for a single commit, as they obfuscate which are the actual 
code changes.  This happened, for example, in main().  Presumably, this 
happened since your editor applied gratiuitous changes "under the hood".

I would prefer if you could resubmit the patch with just the actual changes 
only.  If that's not (easily) possible for you, I'll try to segregate them, but 
it'll take longer then to integrate it.

There's one change I cannot follow at a first glance: In jtag3.c, there is:


@@ -1239,7 +1243,7 @@
 #if !defined(HAVE_LIBUSB)
   avrdude_message(MSG_INFO, "avrdude was compiled without usb support.\n");
   return -1;
-#endif
+#else
 
   if (strncmp(port, "usb", 3) != 0) {
     avrdude_message(MSG_INFO, "%s: jtag3_open_common(): JTAGICE3/EDBG port 
names must start with \"usb\"\n", @@ -1298,6 +1302,7 @@
   jtag3_drain(pgm, 0);
 
   return 0;
+#endif
 }


The #ifdef here is about HAVE_LIBUSB, but why has the #endif been moved?

This change in flip2.c:


+#ifdef _MSC_VER
+    (char*)ptr += read_size;
+#else
     ptr += read_size;
+#endif
+


should probably be written differently.  Even though GCC allows to treat a 
void* like a char* (so it can be incremented), I'd consider that at least poor 
style in the original code, and it should rather be changed to an unsigned 
char* completely, so no #ifdef is needed.

Likewise, in stk500.c:


+#ifdef _MSC_VER
+  unsigned char* buf = _alloca(page_size + 16); #else
   unsigned char buf[page_size + 16];
+#endif


The VLA solution you've chosen for MSC could be used on GCC as well, so there's 
no need for an explicit call to alloca().

Thanks for the submission!


    _______________________________________________________

Reply to this item at:

  
<https://na01.safelinks.protection.outlook.com/?url=http%3a%2f%2fsavannah.nongnu.org%2fpatch%2f%3f8790&data=01%7c01%7cwesw%40microsoft.com%7c677583fe315f4c8d0da208d2ee5c85ec%7c72f988bf86f141af91ab2d7cd011db47%7c1&sdata=ARPaKl0NwHSPEliCpPW1hYVh4k%2fAve0gSK8um%2fECXjE%3d>

_______________________________________________
  Message sent via/by Savannah
  
https://na01.safelinks.protection.outlook.com/?url=http%3a%2f%2fsavannah.nongnu.org%2f&data=01%7c01%7cwesw%40microsoft.com%7c677583fe315f4c8d0da208d2ee5c85ec%7c72f988bf86f141af91ab2d7cd011db47%7c1&sdata=Xa6hUDyX2yI0GF7Y2EY2OyPIsJBnYJIlQn0n02FPpMI%3d