[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

gEDA-cvs: pcb.git: branch: master updated (c1f85a94d591c58ac0f9f6ea4d217f96d14a4d98)



The branch, master has been updated
       via  c1f85a94d591c58ac0f9f6ea4d217f96d14a4d98 (commit)
       via  bdc22fcddac7ae0702acb61c4a55c5397aaf9531 (commit)
       via  65cc2455f59b12e5726a993685e741d1130398a1 (commit)
       via  c54da70385b15a0b19b49ea25354ec8f28dafa09 (commit)
      from  2756468cf3ada09fa768bae8287b73fe79b41a8a (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.


=========
 Summary
=========

 src/action.c              |   15 ++++---
 src/change.c              |    4 +-
 src/hid/batch/batch.c     |  101 +++++----------------------------------------
 src/hid/common/hidnogui.c |   95 +++++++++++++++++++++++++++++++++++-------
 src/vendor.c              |   15 ++++---
 5 files changed, 111 insertions(+), 119 deletions(-)


=================
 Commit Messages
=================

commit c1f85a94d591c58ac0f9f6ea4d217f96d14a4d98
Author: Peter Clifton <pcjc2@xxxxxxxxx>
Commit: Peter Clifton <pcjc2@xxxxxxxxx>

    Plug some memory leaks of strings returned from gui->prompt_for()

:100644 100644 07b2752... 9e1370b... M	src/action.c
:100644 100644 df7f454... 2545d84... M	src/change.c

commit bdc22fcddac7ae0702acb61c4a55c5397aaf9531
Author: Peter Clifton <pcjc2@xxxxxxxxx>
Commit: Peter Clifton <pcjc2@xxxxxxxxx>

    vendor.c: Fix memory leak and unused variable in ActionLoadVendorFrom()
    
    Also, don't bother testing x != NULL before calling free (x).

:100644 100644 39359cc... a296aa1... M	src/vendor.c

commit 65cc2455f59b12e5726a993685e741d1130398a1
Author: Peter Clifton <pcjc2@xxxxxxxxx>
Commit: Peter Clifton <pcjc2@xxxxxxxxx>

    hid/common: Fix / re-write nogui_{prompt_for,fileselect,confirm_dialog}()
    
    Fixes some pretty nasty bugs in these functions:
    
    1. These functions must return allocated memory, strdup will do nicely.
    
    2. fgets will insert the newline character into the buffer, so we need
       to check if we just got a '\r' or '\n' as our first character, not
        just '\0' when deciding whether to return the default string or not.
    
    3. DO NOT strcpy a the default string...
       we don't know if it will overflow our buffer
    
    4. For the "fileselect" case, return NULL if the user didn't give us a
       filename, and the caller didn't specify a default string. Prompt for
       will return strdup (""), equivalent to what it previously did.

:100644 100644 6111ec3... 08adb26... M	src/hid/common/hidnogui.c

commit c54da70385b15a0b19b49ea25354ec8f28dafa09
Author: Peter Clifton <pcjc2@xxxxxxxxx>
Commit: Peter Clifton <pcjc2@xxxxxxxxx>

    hid/batch: Use hidnogui as a default base hid to avoid code duplication

:100644 100644 9715c09... 1fa97c7... M	src/hid/batch/batch.c

=========
 Changes
=========

commit c1f85a94d591c58ac0f9f6ea4d217f96d14a4d98
Author: Peter Clifton <pcjc2@xxxxxxxxx>
Commit: Peter Clifton <pcjc2@xxxxxxxxx>

    Plug some memory leaks of strings returned from gui->prompt_for()

diff --git a/src/action.c b/src/action.c
index 07b2752..9e1370b 100644
--- a/src/action.c
+++ b/src/action.c
@@ -1408,8 +1408,6 @@ NotifyMode (void)
 		    Draw ();
 		  }
 		}
-
-	    /* free memory allocated by gui->prompt_for() */
 	    free (string);
 	  }
 	break;
@@ -4651,7 +4649,8 @@ ActionChangeName (int argc, char **argv, int x, int y)
 	case F_Layout:
 	  name =
 	    gui->prompt_for (_("Enter the layout name:"), EMPTY (PCB->Name));
-	  if (name && ChangeLayoutName (name))	/* XXX memory leak */
+	  /* NB: ChangeLayoutName takes ownership of the passed memory */
+	  if (name && ChangeLayoutName (name))
 	    SetChangedFlag (true);
 	  break;
 
@@ -4659,7 +4658,8 @@ ActionChangeName (int argc, char **argv, int x, int y)
 	case F_Layer:
 	  name = gui->prompt_for (_("Enter the layer name:"),
 				  EMPTY (CURRENT->Name));
-	  if (name && ChangeLayerName (CURRENT, name))	/* XXX memory leak */
+	  /* NB: ChangeLayerName takes ownership of the passed memory */
+	  if (name && ChangeLayerName (CURRENT, name))
 	    SetChangedFlag (true);
 	  break;
 	}
@@ -5951,7 +5951,8 @@ ActionNew (int argc, char **argv, int x, int y)
       CreateNewPCBPost (PCB, 1);
 
       /* setup the new name and reset some values to default */
-      PCB->Name = name;		/* XXX memory leak */
+      free (PCB->Name);
+      PCB->Name = name;
 
       ResetStackAndVisibility ();
       CreateDefaultFont ();
@@ -7605,7 +7606,7 @@ ActionImport (int argc, char **argv, int x, int y)
 
   if (mode && strcasecmp (mode, "setdisperse") == 0)
     {
-      const char *ds, *units;
+      char *ds, *units;
       char buf[50];
 
       ds = ARG (1);
@@ -7622,6 +7623,8 @@ ActionImport (int argc, char **argv, int x, int y)
 	}
       else
 	AttributePut (PCB, "import::disperse", ds);
+      if (ARG (1) == NULL)
+        free (ds);
       return 0;
     }
 
diff --git a/src/change.c b/src/change.c
index df7f454..2545d84 100644
--- a/src/change.c
+++ b/src/change.c
@@ -1079,6 +1079,7 @@ ChangeTextName (LayerTypePtr Layer, TextTypePtr Text)
 bool
 ChangeLayoutName (char *Name)
 {
+  free (PCB->Name);
   PCB->Name = Name;
   hid_action ("PCBChanged");
   return (true);
@@ -1106,6 +1107,7 @@ ChangeElementSide (ElementTypePtr Element, LocationType yoff)
 bool
 ChangeLayerName (LayerTypePtr Layer, char *Name)
 {
+  free (CURRENT->Name);
   CURRENT->Name = Name;
   hid_action ("LayersChanged");
   return (true);
@@ -2280,7 +2282,7 @@ QueryInputAndChangeObjectName (int Type, void *Ptr1, void *Ptr2, void *Ptr3)
     }
   if (name)
     {
-      /* XXX Memory leak!! */
+      /* NB: ChangeObjectName takes ownership of the passed memory */
       char *old = ChangeObjectName (Type, Ptr1, Ptr2, Ptr3, name);
       if (old != (char *) -1)
 	{

commit bdc22fcddac7ae0702acb61c4a55c5397aaf9531
Author: Peter Clifton <pcjc2@xxxxxxxxx>
Commit: Peter Clifton <pcjc2@xxxxxxxxx>

    vendor.c: Fix memory leak and unused variable in ActionLoadVendorFrom()
    
    Also, don't bother testing x != NULL before calling free (x).

diff --git a/src/vendor.c b/src/vendor.c
index 39359cc..a296aa1 100644
--- a/src/vendor.c
+++ b/src/vendor.c
@@ -269,11 +269,12 @@ int
 ActionLoadVendorFrom (int argc, char **argv, int x, int y)
 {
   int i;
-  char *fname = NULL, *name = NULL;
+  char *fname = NULL;
   static char *default_file = NULL;
   char *sval;
   Resource *res, *drcres, *drlres;
   int type;
+  bool free_fname = false;
 
   cached_drill = -1;
 
@@ -291,11 +292,10 @@ ActionLoadVendorFrom (int argc, char **argv, int x, int y)
       if (fname == NULL)
 	AFAIL (load_vendor);
 
-      if (default_file != NULL)
-	{
-	  free (default_file);
-	  default_file = NULL;
-	}
+      free_fname = true;
+
+      free (default_file);
+      default_file = NULL;
 
       if (fname && *fname)
 	default_file = strdup (fname);
@@ -459,7 +459,8 @@ ActionLoadVendorFrom (int argc, char **argv, int x, int y)
 
   vendorMapEnable = true;
   apply_vendor_map ();
-  free (name);
+  if (free_fname)
+    free (fname);
   return 0;
 }
 

commit 65cc2455f59b12e5726a993685e741d1130398a1
Author: Peter Clifton <pcjc2@xxxxxxxxx>
Commit: Peter Clifton <pcjc2@xxxxxxxxx>

    hid/common: Fix / re-write nogui_{prompt_for,fileselect,confirm_dialog}()
    
    Fixes some pretty nasty bugs in these functions:
    
    1. These functions must return allocated memory, strdup will do nicely.
    
    2. fgets will insert the newline character into the buffer, so we need
       to check if we just got a '\r' or '\n' as our first character, not
        just '\0' when deciding whether to return the default string or not.
    
    3. DO NOT strcpy a the default string...
       we don't know if it will overflow our buffer
    
    4. For the "fileselect" case, return NULL if the user didn't give us a
       filename, and the caller didn't specify a default string. Prompt for
       will return strdup (""), equivalent to what it previously did.

diff --git a/src/hid/common/hidnogui.c b/src/hid/common/hidnogui.c
index 6111ec3..08adb26 100644
--- a/src/hid/common/hidnogui.c
+++ b/src/hid/common/hidnogui.c
@@ -264,14 +264,73 @@ nogui_logv (const char *fmt, va_list ap)
   vprintf (fmt, ap);
 }
 
+/* Return a line of user input text, stripped of any newline characters.
+ * Returns NULL if the user simply presses enter, or otherwise gives no input.
+ */
+#define MAX_LINE_LENGTH 1024
+static char *
+read_stdin_line (void)
+{
+  static char buf[MAX_LINE_LENGTH];
+  char *s;
+  int i;
+
+  s = fgets (buf, MAX_LINE_LENGTH, stdin);
+  if (s == NULL)
+    {
+      printf ("\n");
+      return NULL;
+    }
+
+  /* Strip any trailing newline characters */
+  for (i = strlen (s) - 1; i >= 0; i--)
+    if (s[i] == '\r' || s[i] == '\n')
+      s[i] = '\0';
+
+  if (s[0] == '\0')
+    return NULL;
+
+  return strdup (s);
+}
+#undef MAX_LINE_LENGTH
+
 static int
 nogui_confirm_dialog (char *msg, ...)
 {
-  int rv;
-  printf ("%s ? 0=cancel 1=ok : ", msg);
-  fflush (stdout);
-  scanf ("%d", &rv);
-  return rv;
+  char *answer;
+  int ret = 0;
+  bool valid_answer = false;
+  va_list args;
+
+  do
+    {
+      va_start (args, msg);
+      vprintf (msg, args);
+      va_end (args);
+
+      printf (" ? 0=cancel 1 = ok : ");
+
+      answer = read_stdin_line ();
+
+      if (answer == NULL)
+        continue;
+
+      if (answer[0] == '0' && answer[1] == '\0')
+        {
+          ret = 0;
+          valid_answer = true;
+        }
+
+      if (answer[0] == '1' && answer[1] == '\0')
+        {
+          ret = 1;
+          valid_answer = true;
+        }
+
+      free (answer);
+    }
+  while (!valid_answer);
+  return ret;
 }
 
 static int
@@ -289,15 +348,18 @@ nogui_report_dialog (char *title, char *msg)
 static char *
 nogui_prompt_for (const char *msg, const char *default_string)
 {
-  static char buf[1024];
+  char *answer;
+
   if (default_string)
     printf ("%s [%s] : ", msg, default_string);
   else
     printf ("%s : ", msg);
-  fgets (buf, 1024, stdin);
-  if (buf[0] == 0 && default_string)
-    strcpy (buf, default_string);
-  return buf;
+
+  answer = read_stdin_line ();
+  if (answer == NULL)
+    return strdup ((default_string != NULL) ? default_string : "");
+  else
+    return strdup (answer);
 }
 
 /* FIXME - this could use some enhancement to actually use the other
@@ -307,15 +369,18 @@ nogui_fileselect (const char *title, const char *descr,
 		  char *default_file, char *default_ext,
 		  const char *history_tag, int flags)
 {
-  static char buf[1024];
+  char *answer;
+
   if (default_file)
     printf ("%s [%s] : ", title, default_file);
   else
     printf ("%s : ", title);
-  fgets (buf, 1024, stdin);
-  if (buf[0] == 0 && default_file)
-    strcpy (buf, default_file);
-  return buf;
+
+  answer = read_stdin_line ();
+  if (answer == NULL)
+    return (default_file != NULL) ? strdup (default_file) : NULL;
+  else
+    return strdup (answer);
 }
 
 static int

commit c54da70385b15a0b19b49ea25354ec8f28dafa09
Author: Peter Clifton <pcjc2@xxxxxxxxx>
Commit: Peter Clifton <pcjc2@xxxxxxxxx>

    hid/batch: Use hidnogui as a default base hid to avoid code duplication

diff --git a/src/hid/batch/batch.c b/src/hid/batch/batch.c
index 9715c09..1fa97c7 100644
--- a/src/hid/batch/batch.c
+++ b/src/hid/batch/batch.c
@@ -332,73 +332,6 @@ batch_stop_block_hook (hidval mlpoll)
 {
 }
 
-static void
-batch_log (const char *fmt, ...)
-{
-  va_list ap;
-  va_start (ap, fmt);
-  vprintf (fmt, ap);
-  va_end (ap);
-}
-
-static void
-batch_logv (const char *fmt, va_list ap)
-{
-  vprintf (fmt, ap);
-}
-
-static int
-batch_confirm_dialog (char *msg, ...)
-{
-  int rv;
-  printf ("%s ? 0=cancel 1=ok : ", msg);
-  fflush (stdout);
-  scanf ("%d", &rv);
-  return rv;
-}
-
-static int
-batch_close_confirm_dialog ()
-{
-  return batch_confirm_dialog (_("OK to lose data ?"), NULL);
-}
-
-static void
-batch_report_dialog (char *title, char *msg)
-{
-  printf ("--- %s ---\n%s\n", title, msg);
-}
-
-static char *
-batch_prompt_for (const char *msg, const char *default_string)
-{
-  static char buf[1024];
-  if (default_string)
-    printf ("%s [%s] : ", msg, default_string);
-  else
-    printf ("%s : ", msg);
-  fgets (buf, 1024, stdin);
-  if (buf[0] == 0 && default_string)
-    strcpy (buf, default_string);
-  return buf;
-}
-
-static char *
-batch_fileselect (const char *title, const char *descr,
-		  char *default_file, char *default_ext,
-		  const char *history_tag, int flags)
-{
-  static char buf[1024];
-  if (default_file)
-    printf ("%s [%s] : ", title, default_file);
-  else
-    printf ("%s : ", title);
-  fgets (buf, 1024, stdin);
-  if (buf[0] == 0 && default_file)
-    strcpy (buf, default_file);
-  return buf;
-}
-
 static int
 batch_attribute_dialog (HID_Attribute * attrs_,
 			int n_attrs_, HID_Attr_Val * results_,
@@ -412,19 +345,6 @@ batch_show_item (void *item)
 {
 }
 
-static void
-batch_beep (void)
-{
-  putchar (7);
-  fflush (stdout);
-}
-
-static int
-batch_progress (int so_far_, int total_, const char *message_)
-{
-  return 0;
-}
-
 HID batch_gui = {
   sizeof (HID),
   "batch",
@@ -465,18 +385,18 @@ HID batch_gui = {
   batch_unwatch_file,
   batch_add_block_hook,
   batch_stop_block_hook,
-  batch_log,
-  batch_logv,
-  batch_confirm_dialog,
-  batch_close_confirm_dialog,
-  batch_report_dialog,
-  batch_prompt_for,
-  batch_fileselect,
+  0 /* batch_log */,
+  0 /* batch_logv */,
+  0 /* batch_confirm_dialog */,
+  0 /* batch_close_confirm_dialog */,
+  0 /* batch_report_dialog */,
+  0 /* batch_prompt_for */,
+  0 /* batch_fileselect */,
   batch_attribute_dialog,
   batch_show_item,
-  batch_beep,
-  batch_progress,
-  0 /* batch_drc_gui */ ,
+  0 /* batch_beep */,
+  0 /* batch_progress */,
+  0 /* batch_drc_gui */,
   0 /* batch_edit_attributes */
 };
 
@@ -485,6 +405,7 @@ HID batch_gui = {
 void
 hid_batch_init ()
 {
+  apply_default_hid (&batch_gui, 0);
   hid_register_hid (&batch_gui);
 #include "batch_lists.h"
 }




_______________________________________________
geda-cvs mailing list
geda-cvs@xxxxxxxxxxxxxx
http://www.seul.org/cgi-bin/mailman/listinfo/geda-cvs