[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [avrdude-dev] [patch #8790] Add support for Visual Studio Builds
From: |
Wes Witt |
Subject: |
Re: [avrdude-dev] [patch #8790] Add support for Visual Studio Builds |
Date: |
Mon, 16 Nov 2015 17:50:26 +0000 |
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