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

Re: gEDA-user: gsch2pcb use pcb library preferences patch



   Thanks for the comments.  I have made suggested changes.  I have tried
   using g_strsplit and found it to takes more lines of code and uses
   more memory.  I prefer the changes to be walking-style instead of
   g_strsplit due to fewer lines, less allocated memory, and it will be
   easier to make it accept escape characters or other tweaks.  I made a
   function to add libraries that replaces the strtok code and can be
   used if more libraries need to be added.
   -Josh Jordan
   --- On Wed, 3/11/09, Peter Clifton <pcjc2@xxxxxxxxx> wrote:

     From: Peter Clifton <pcjc2@xxxxxxxxx>
     Subject: Re: gEDA-user: gsch2pcb use pcb library preferences patch
     To: "gEDA user mailing list" <geda-user@xxxxxxxxxxxxxx>
     Date: Wednesday, March 11, 2009, 12:38 PM

   On Tue, 2009-03-10 at 17:47 -0700, Josh Jordan wrote:
   > This patch lets gsch2pcb open up ~/.pcb/preferences and read the
   >    library-newlib = paths and add them to the list of places to get
   >    footprints.  The patch is directly for gsch2pcb.c
   If you could send patches readdy for applying, that would be great.
   I'm
   noting that we don't actually want:
   +       printf("going into new code\n");
   +       printf("coming out of new code\n");
   or any other of the live debugging printf patched into gsch2pcb.
   I think the general idea of the patch is fine. We have various plans
   to
   completely re-write how gsch2pcb works at some point (making it defer
   operations to PCB), but in the absence of solid progress on that
   front,
   I don't see how improving the exsting gsch2pcb hurts anything.
   +                       if (g_str_has_prefix(buf, "library-newlib =
   "))
   +                       {
   +                               pref_paths = buf+17;
   Code like this is very dangerous from a maintenance point of view.
   You could use:
   pref_paths = buf + strlen ("library-newlib = ");
   (Bonus points for storing that string in a variable or #define). Smart
   compilers "ought" to be able to optimise out the strlen call of a
   static
   string, and if not.. we're not exactly on a time-critical path here.
   +                                       while(*++tmp != ':' && *tmp !=
   0); //push np up to next : or end of string
   This breaks portability to Win32, where we use ";" as a path
   separator.
   I presume you want to change the "np" in the comment to "tmp"?
   You'll notice that the last context after your diff is:
   #define PCB_PATH_DELIMETER ":"
   This suggests that we already had a portability problem there. In any
   case, you should put your changes after that define, and use it rather
   than hard-coding the delimiter.
   I doubt any of our existing code gracefully handles ":" or ";" in
   filenames, so I'm not going to gripe that yours doesn't allow any
   escaping of those characters to treat them as part of the file-names.
   Given that you're not handling escaping (which might require you to
   walk
   the string manually), g_strsplit() might be a neater way to dice up
   the
   path. (It would be more easy to visually inspect for correctness than
   traditional pointer arithmetic based string walking.)
   I note that you're not using similar code to that existing in gsch2pcb
   to parse the path string (just after your changes). That code uses
   strtok, however the man page for that routine suggests it should be
   avoided for various reasons. (IE.. don't use it in your new code).
   A patch to make both code blocks use the same method (whatever that
   is)
   would be welcome. I'll also note that they could probably be
   refactored
   to share that common code (once you've stripped off the
   "library-newlib
   = " prefix (using pointer arithmetic).
   Stylistic points.. Your code's indenting doesn't seem to match the
   existing gsch2pcb code (or the typical convention used in the rest of
   the main gEDA code-base). Additionally, the gEDA code base does not
   generally use "//" comments. Instead, use /* ..... */ blocks.
   I hope these comments are useful, and help you improve the patches
   further. it is always nice to have new people working on gEDA / PCB,
   and
   this is certainly an area which hasn't seen a lot of active
   maintenance
   for a while now.
   Best regards,
   --
   Peter Clifton
   Electrical Engineering Division,
   Engineering Department,
   University of Cambridge,
   9, JJ Thomson Avenue,
   Cambridge
   CB3 0FA
   Tel: +44 (0)7729 980173 - (No signal in the lab!)
   _______________________________________________
   geda-user mailing list
   [1]geda-user@xxxxxxxxxxxxxx
   [2]http://www.seul.org/cgi-bin/mailman/listinfo/geda-user

References

   1. file://localhost/mc/compose?to=geda-user@xxxxxxxxxxxxxx
   2. http://www.seul.org/cgi-bin/mailman/listinfo/geda-user
--- gsch2pcb_orig.c	2009-03-13 00:37:48.000000000 -0400
+++ gsch2pcb.c	2009-03-13 00:36:39.000000000 -0400
@@ -1229,6 +1229,32 @@
 	return 1;
 	}
 
+#define PCB_PATH_DELIMETER ":"
+static void
+add_library_paths(gchar *paths)
+	{
+	gchar *tmp, *path, *epath;
+	tmp = g_strchomp(paths);
+	while(*tmp)
+		{
+		paths = tmp;
+		while(*tmp++ != PCB_PATH_DELIMETER[0] && *tmp);
+		if(*(tmp-1) == PCB_PATH_DELIMETER[0])
+			path = g_strndup(paths, strlen(paths)-strlen(tmp)-1);
+		else
+			path = g_strndup(paths, strlen(paths)-strlen(tmp));
+		epath = expand_dir(path);
+		g_free(path);
+		if (g_file_test(epath, G_FILE_TEST_IS_DIR))
+			{
+			element_directory_list = g_list_append(element_directory_list, g_strdup(epath));
+			if (verbose)
+				printf("added library path- %s\n", epath);
+			}
+		g_free(epath);
+		}
+	}
+
 static void
 load_project(gchar *path)
 	{
@@ -1468,22 +1494,32 @@
 		element_directory_list = g_list_append(element_directory_list,
 					"packages");
 
-#define PCB_PATH_DELIMETER ":"
-	if (verbose)
-		printf ("Processing PCBLIBPATH=\"%s\"\n", PCBLIBPATH);
-
-	path = g_strdup (PCBLIBPATH);
-	for (p = strtok (path, PCB_PATH_DELIMETER); p && *p;
-		p = strtok (NULL, PCB_PATH_DELIMETER))
+	#define GTK_LIB_PREF_KEY "library-newlib = "
+	/* load library paths from pcb preferences ~/.pcb/preferences */
+	path = expand_dir("~/.pcb/preferences");
+	if (g_file_test(path, G_FILE_TEST_IS_REGULAR))
 	  {
-		if (g_file_test(p, G_FILE_TEST_IS_DIR))
-		  {
-			if (verbose)
-				printf ("Adding %s to the newlib search path\n", p);
-			element_directory_list = g_list_append(element_directory_list,
-					g_strdup(p));
+		char buf[1024];
+		gchar *pref_paths;
+		FILE *fp;
+		fp=fopen(path, "r");
+		while(fgets(buf, sizeof(buf), fp)) 
+		  { 
+			if (g_str_has_prefix(buf, GTK_LIB_PREF_KEY))
+			  {
+				pref_paths = g_strchomp(buf + strlen(GTK_LIB_PREF_KEY));
+				if (verbose)
+					printf ("Processing %s\"%s\"\n", GTK_LIB_PREF_KEY, pref_paths);
+				add_library_paths(pref_paths);
+			  }       
 		  }
+		fclose(fp);
 	  }
+
+	if (verbose)
+		printf ("Processing PCBLIBPATH=\"%s\"\n", PCBLIBPATH);
+	path = g_strdup (PCBLIBPATH);
+	add_library_paths(path);
 	g_free (path);
 
 	pins_file_name = g_strconcat(basename, ".cmd", NULL);

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