avrdude-dev
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Joerg Wunsch
Subject: [avrdude-dev] [patch #8790] Add support for Visual Studio Builds
Date: Mon, 16 Nov 2015 08:04:14 +0000
User-agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:37.0) Gecko/20100101 Firefox/37.0

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:

  <http://savannah.nongnu.org/patch/?8790>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.nongnu.org/




reply via email to

[Prev in Thread] Current Thread [Next in Thread]