Index: doc-rnd/devlog/20160316_unglib.html =================================================================== --- doc-rnd/devlog/20160316_unglib.html (revision 1286) +++ doc-rnd/devlog/20160316_unglib.html (nonexistent) @@ -1,49 +0,0 @@ - - -

pcb-rnd devlog

- -

unglib: glib removal

- -

glib problems

- -As mentioned in a previous post, glib is -sometimes dangerous. Even if it was not, it's still huge and contradicts -the UNIX philosophy: Do One Thing and Do It Well. Glib tries to do a lot -of things. I do not doubt it is trying to do each of these things well, but as -a combination it's pretty much impossible to Keep It Simple, Stupid. -While glib is modular in design, from the viewpoint of an application it is -not modular: it's highly unlikely that the application can replace a part -of glib while keeping the rest. -

-The source of these problems is that glib is a "megalib" that tries to solve -a host of problems as a package. - -

The solution

- -The solution is to replace the megalib with -a set of independent minilibs. Each minilib: -

tries to do one thing - e.g. linked lists, without coupling it with a custom memory allocator -
is simple - the API is so small that it's easy to learn it in minutes -
is small - so that if anything breaks it's very easy to find and fix the bug (when did you last debug internals of glib?) -
is replaceable - since they are independent, if one doesn't work up to expectations, it's real easy to replace it without affecting any other part; e.g. replacing linked lists without replacing hash tables -

-The minilibs are imported as svn externals in trunk/src_3rd. They are small -enough so that they can be distributed together with pcb sources. - -

Current state

- -The "unglib" patch is mostly done. All references of glib are removed -from the core and the lesstif hid. There are a three components that -still depend on glib, but they each can be disabled: -

the GTK HID: because gtk is already coupled with glib, it doesn't make much sense to remove glib from the gtk HID code -
the toporouter plugin: the code is huge and will be potentially deprecated anyway (lack of developer resources/user interest) -
the puller: glib should be removed from the puller long term but the code is big and there's no much user need for it in pcb-rnd -

-This means pcb-rnd can be compiled with lesstif (or a compatible motif) -on a UNIX box without depending on glib. Together with the earlier effort -that removed autotools, it means a UNIX box without any "GNU infection" -should be able to compile and run pcb-rnd. Index: doc-rnd/devlog/20160313_unglib.html =================================================================== --- doc-rnd/devlog/20160313_unglib.html (nonexistent) +++ doc-rnd/devlog/20160313_unglib.html (revision 1287) @@ -0,0 +1,49 @@ + + +

pcb-rnd devlog

+ +

unglib: glib removal

+ +

glib problems

+ +As mentioned in a previous post, glib is +sometimes dangerous. Even if it was not, it's still huge and contradicts +the UNIX philosophy: Do One Thing and Do It Well. Glib tries to do a lot +of things. I do not doubt it is trying to do each of these things well, but as +a combination it's pretty much impossible to Keep It Simple, Stupid. +While glib is modular in design, from the viewpoint of an application it is +not modular: it's highly unlikely that the application can replace a part +of glib while keeping the rest. +

+The source of these problems is that glib is a "megalib" that tries to solve +a host of problems as a package. + +

The solution

+ +The solution is to replace the megalib with +a set of independent minilibs. Each minilib: +

tries to do one thing - e.g. linked lists, without coupling it with a custom memory allocator +
is simple - the API is so small that it's easy to learn it in minutes +
is small - so that if anything breaks it's very easy to find and fix the bug (when did you last debug internals of glib?) +
is replaceable - since they are independent, if one doesn't work up to expectations, it's real easy to replace it without affecting any other part; e.g. replacing linked lists without replacing hash tables +

+The minilibs are imported as svn externals in trunk/src_3rd. They are small +enough so that they can be distributed together with pcb sources. + +

Current state

+ +The "unglib" patch is mostly done. All references of glib are removed +from the core and the lesstif hid. There are a three components that +still depend on glib, but they each can be disabled: +

the GTK HID: because gtk is already coupled with glib, it doesn't make much sense to remove glib from the gtk HID code +
the toporouter plugin: the code is huge and will be potentially deprecated anyway (lack of developer resources/user interest) +
the puller: glib should be removed from the puller long term but the code is big and there's no much user need for it in pcb-rnd +

+This means pcb-rnd can be compiled with lesstif (or a compatible motif) +on a UNIX box without depending on glib. Together with the earlier effort +that removed autotools, it means a UNIX box without any "GNU infection" +should be able to compile and run pcb-rnd. Index: doc-rnd/devlog/20160314_valgrind_flex.html =================================================================== --- doc-rnd/devlog/20160314_valgrind_flex.html (nonexistent) +++ doc-rnd/devlog/20160314_valgrind_flex.html (revision 1287) @@ -0,0 +1,79 @@ + + +

pcb-rnd devlog

+ +

"valgrinding" flex/bison parsers

+ +In a flex/bison parser it's quiet common that strings are allocated +in flex, passed on to bison and then either free'd there or saved in +the output data. Since both free and save happens a lot, it's not an +easy mechanical review of the .y file to find the reason for leaks. Especially +if the code has to store some strings in temporary storage until a later +stage of the parsing. +

+A typical valgrind log for such a leak looks like this: +

+==20520== 20 bytes in 1 blocks are still reachable in loss record 3 of 6
+==20520==    at 0x402B0D5: calloc (vg_replace_malloc.c:623)
+==20520==    by 0x80E6EF0: yylex (parse_l.l:177)
+==20520==    by 0x80E0D6C: yyparse (parse_y.tab.c:1696)
+==20520==    by 0x80E85ED: Parse (parse_l.l:292)
+==20520==    by 0x80E876B: ParsePCB (parse_l.l:347)
+==20520==    by 0x8078591: real_load_pcb (file.c:390)
+==20520==    by 0x80787E9: LoadPCB (file.c:459)
+==20520==    by 0x8097719: main (main.c:1781)
+

+ +The code at parse_l.l:177 is just a calloc() and some string +operation: this is where the string is created. The STRING token is +referenced about 58 times in the grammar. After reading through the +whole file 4..5 times, I still didn't see any obvious place for the leak. +

+The leak was also a rare one: happened for one string per file. This suggested +it was in the header - unless there's an one-instance object somewhere in the +.pcb or it's a cache where the same pointer is free()'d and overwritten for +multiple occurrences and simply no one free()'s the last. +

+Assuming it's a header, a cheap ways to find which header field leaked: +

remove the header lines one by one and see if the leak is still there - won't work if a missing header changes how the whole code runs +
change the size of each header, one by one, and see which action changes leak size - a lot of iterations, and valgrinding is slow +

+At this point I figured that I'd depend on the reported size of the leak +with my tests. I didn't want to do multiple runs and didn't want to risk +the whole parser to run differently so I didn't want to modify the input. Instead +I figured there's a simple, yet generic way to track these sort of leaks. +

+I estimated no string in the file is longer than 1000 characters. Right above +the calloc() in the lexer I introduced a new static integer variable starting +at 1000, increased before each allocation. This counter is sort of an ID of +each allocation. Then I modified the calloc() to ignore the originally calculated +string length and use this ID for allocation size. I also printed the ID-string +pairs. The original code looked like this (simplified): +

+	/* ... predict string size ... */
+	yylval.str = calloc(predicted_size, 1);
+	/* ... build the string here ... */
+	return STRING;
+

+ +The resulting code looked like this (simplified): +

+	/* ... predict string size ... */
+	static int alloc_id = 1000;
+	alloc_id++;
+	yylval.str = calloc(alloc_id, 1);
+	/* ... build the string here ... */
+	fprintf(stderr, "STRING: %d '%s'\n", alloc_id, yylval.str);
+	return STRING;
+

+I saved the list printed on stderr and checked valgrind's log to find +the two strings in question were ID 1002 and ID 1007, both looked +something like this: +

+1,3,4,c:2,5,6,s:7:8
+

+The only thing that looks like this is the layer group description ("Groups()"). +From this point it was trivial to find the bug in the grammar. +