| 00:12.56 | *** join/#brlcad cwstirk (~charlie@c-71-56-216-45.hsd1.co.comcast.net) | |
| 00:14.40 | *** join/#brlcad cwstirk (~charlie@c-71-56-216-45.hsd1.co.comcast.net) | |
| 00:53.06 | *** join/#brlcad FreezingCold (~FreezingC@135.0.41.14) | |
| 01:37.35 | *** join/#brlcad kintel (~kintel@unaffiliated/kintel) | |
| 01:55.57 | *** join/#brlcad Zhao_Anqing (~clouddrif@123.157.213.219) | |
| 02:18.00 | *** join/#brlcad ries (~ries@190.9.171.121) | |
| 03:00.16 | *** join/#brlcad ries (~ries@190.9.171.121) | |
| 03:01.56 | *** join/#brlcad FreezingCold (~FreezingC@135.0.41.14) | |
| 03:34.01 | *** join/#brlcad teepee (~teepee@gateway/tor-sasl/teepee) | |
| 03:38.56 | *** join/#brlcad kintel (~kintel@unaffiliated/kintel) | |
| 04:37.14 | *** join/#brlcad caen23 (~caen23@109.97.108.82) | |
| 04:40.34 | *** join/#brlcad witness___ (uid10044@gateway/web/irccloud.com/x-knlqdlzssojfgwsy) | |
| 04:54.11 | Notify | 03BRL-CAD Wiki:Shreesathyam * 0 /wiki/User:Shreesathyam: |
| 05:21.09 | *** join/#brlcad ries (~ries@190.9.171.121) | |
| 07:26.43 | *** join/#brlcad luca79 (~luca@net-37-116-124-236.cust.vodafonedsl.it) | |
| 07:32.15 | *** join/#brlcad caen23 (~caen23@109.97.108.82) | |
| 08:05.04 | *** join/#brlcad KimK_ (~Kim__@ip68-102-30-143.ks.ok.cox.net) | |
| 08:26.34 | *** join/#brlcad d_rossberg (~rossberg@66-118-151-70.static.sagonet.net) | |
| 08:55.09 | *** join/#brlcad Anaphaxeton (~george@ppp005054109224.access.hol.gr) | |
| 09:00.24 | *** join/#brlcad Anaphaxet0n (~george@ppp046176099234.access.hol.gr) | |
| 09:02.20 | *** join/#brlcad teepee- (bc5c2133@gateway/web/freenode/ip.188.92.33.51) | |
| 09:09.56 | *** join/#brlcad ``Erik (~erik@pool-74-103-94-19.bltmmd.fios.verizon.net) | |
| 10:29.46 | *** join/#brlcad teepee- (bc5c2133@gateway/web/freenode/ip.188.92.33.51) | |
| 11:03.46 | *** join/#brlcad Zhao_Anqing (~clouddrif@183.157.160.7) | |
| 13:07.54 | *** join/#brlcad ries (~ries@190.9.171.121) | |
| 13:20.49 | *** join/#brlcad luca79 (~luca@net-37-116-124-236.cust.vodafonedsl.it) | |
| 13:20.55 | *** join/#brlcad teepee- (bc5c2133@gateway/web/freenode/ip.188.92.33.51) | |
| 13:35.39 | brlcad | starseeker: don't think so, merges from RELEASE to trunk should be rare/nonexistent so there's not much concern when it's a simple compilation fix, only when it's substantial |
| 13:41.03 | brlcad | Zhao_Anqing: also note in HACKING that calling free() was probably not just the lesser choice given nmg_km() but probably wrong |
| 13:42.11 | *** join/#brlcad FreezingCold (~FreezingC@135.0.41.14) | |
| 13:42.13 | brlcad | you'll always want to think about where (all) memory comes from, how it was allocated, who is responsible for freeing it, and how it should be deallocated |
| 13:44.27 | brlcad | free() only pairs with malloc()/calloc()/realloc() ... so you would have had to confirm that 1) it was actually allocated with malloc/calloc/realloc, 2) that you are responsible for freeing it (how do you know this?), and 3) calling whatever routine necessary as determined by #1 if #2 is true |
| 13:50.02 | *** join/#brlcad infobot (~infobot@rikers.org) | |
| 13:50.02 | *** topic/#brlcad is BRL-CAD || http://brlcad.org || logs: http://ibot.rikers.org/%23brlcad/ || GCI winners: Jacob Burroughs and Peter Amidon! || GSoC 2014 selections are announced! Thank you to all we got to work with. Remember that SOCIS is coming up right around the corner and you don't need a summer of code to get involved with open source. | |
| 13:50.32 | Zhao_Anqing | brlcad: Ok. I will remember this tip. Thank you sincerely. |
| 13:52.20 | Zhao_Anqing | Actually, I have a bit hesitate when I use free() just now. But I still...Aha. However, I will read the HACK file again, more carefully. |
| 13:52.33 | brlcad | Zhao_Anqing: and my question? :) |
| 13:52.42 | brlcad | (how do you know this?) |
| 13:54.54 | brlcad | starseeker: comb -S doesn't seem to actually do anything? |
| 13:56.01 | Zhao_Anqing | brlcad: is it true that I sould free all pointer malloced by myself? |
| 13:57.20 | brlcad | Zhao_Anqing: eh, no ... "it depends" |
| 13:57.28 | brlcad | it depends on who is responsible for freeing it |
| 13:57.48 | brlcad | so again, my question ... how do you know whether you are responsible for freeing it or not? |
| 14:04.50 | Zhao_Anqing | brlcad: er..Sorry.I have no idea about this. Maybe I could ask the writer of this routine? |
| 14:06.04 | Zhao_Anqing | If this pointer won't be used in other place, it should be freed? |
| 14:12.38 | *** join/#brlcad kintel (~kintel@unaffiliated/kintel) | |
| 14:13.18 | brlcad | Zhao_Anqing: you definitely could ask the writer of the routine |
| 14:13.39 | brlcad | that doesn't scale very well though .. how might the writer have indicated who is responsible? |
| 14:14.18 | Notify | 03BRL-CAD:brlcad * 60519 brlcad/trunk/src/libged/comb.c: I think we're meant to exit here? otherwise, the check and -S flag seem to do nothing. |
| 14:14.40 | brlcad | or more practically speaking, so you can't get ahold of the writer or don't know how to get ahold of them, how else might you figure out who is responsible? |
| 14:15.49 | starseeker | brlcad: I think I ment that as a "safe" flag |
| 14:16.33 | *** join/#brlcad teepee- (bc5c2133@gateway/web/freenode/ip.188.92.33.51) | |
| 14:17.11 | starseeker | looks like you're right though, I didn't return |
| 14:17.12 | starseeker | oops |
| 14:18.47 | starseeker | grr nmg_misc.c:130:42: error: array subscript is below array bounds [-Werror=array-bounds] |
| 14:19.08 | starseeker | not sure what to make of that - looks like >> operator bit shifting at work |
| 14:20.34 | starseeker | ``Erik's kind of stuf |
| 14:20.36 | starseeker | stuff even |
| 14:21.44 | starseeker | part of the old NURBS code... |
| 14:22.03 | starseeker | well, that's what we get for not merging that functionality in and clearing it out... |
| 14:22.39 | Zhao_Anqing | brlcad: is it true..If I create a pointer in routine and it's also not a return value. I should free it. |
| 14:23.11 | starseeker | wonders if that would be a good GCI task - list out our old NURBS functionality, map it to the existing (or not) openNURBS versions, and see what's missing, what's better/worse, etc. |
| 14:25.51 | Zhao_Anqing | if the pointer is an input parameter..it depends..just like nmg_mrsv, I needn't free the input m in it. |
| 14:26.28 | ``Erik | heh |
| 14:27.08 | ``Erik | starseeker: boiled down, it looks like "coords=0; crv_pt[coords-1];" ... |
| 14:29.17 | Notify | 03BRL-CAD:brlcad * 60520 brlcad/trunk/doc/docbook/system/mann/en/comb.xml: document the -S option |
| 14:29.44 | ``Erik | at first glance, anyway... mebbe stick some guards in there or disable the werror, then whack it with a debugger to see where it gets the negative index from |
| 14:30.50 | Notify | 03BRL-CAD:brlcad * 60521 brlcad/trunk/NEWS: cliff added the -S option to comb command that makes it behave similar to the 'c' command, stopping if the object already exists |
| 14:35.16 | Notify | 03BRL-CAD:brlcad * 60522 brlcad/trunk/src/libged/comb.c: this a dead code condition. two flaws in a single commit.. possibly more. coverity would have caught one of them but is that a solution? |
| 14:36.05 | brlcad | starseeker: I know you did, but you don't define safe and it could literally mean any number of things |
| 14:37.40 | brlcad | also, verb/action vs adjective/description |
| 14:39.19 | brlcad | actions tend to not need much explanation, beyond what case triggers the action |
| 14:40.32 | brlcad | the bigger issue I have is whether we need that option at all, it would probably make more sense as the default behavior with a flag to append/modify an existing |
| 14:44.01 | brlcad | Zhao_Anqing: you could create a pointer in a routine and indirectly return it (e.g., via a global or via some struct field, or passing it to some other function that stores it, etc) |
| 14:44.21 | brlcad | rather, it's not being returned, but there are references to it |
| 14:44.44 | brlcad | so the answer is still "it depends" |
| 14:45.15 | brlcad | Zhao_Anqing: my question to you is how do you know when to free it and when to not, whether you need to free it? |
| 14:46.05 | brlcad | Zhao_Anqing: say you see a line of code like this, how do you know? char *my_var = zhao_sean_interface(); |
| 14:46.45 | brlcad | it's wrong to assume you need to call free(my_var) |
| 14:46.54 | brlcad | it's wrong to assume you don't need to free my_var |
| 14:50.31 | Zhao_Anqing | brlcad: @_@ Sorry Sean, I really don't know the answer. Teach me please. |
| 14:51.43 | d_rossberg | Zhao_Anqing: an example from your patch: "struct model *m = nmg_mm();" vs. "struct shell *s = nmg_msv(r);" |
| 14:52.01 | Zhao_Anqing | the comment of this routine should indicate whether I should free it or not? |
| 14:53.09 | brlcad | Zhao_Anqing: now you're thinking it through! yes, it should be documented somewhere or implied strongly via some convention |
| 14:53.38 | Zhao_Anqing | I read the codes in nmg_km and nmg_kr, then I am sure I just need to call nmg_km, but not nmg_kr. |
| 14:54.02 | Zhao_Anqing | because nmg_km contain freeing region struct |
| 14:55.16 | brlcad | you should see that it's getting a pointer back from nmg_mm(), so you look for documentation on that function ... |
| 14:55.26 | brlcad | that should be documented where it's declared in include/raytrace.h but it's not (problem #1) |
| 14:55.54 | brlcad | looking where it's defined, in src/librt/primitives/nmg, there is some helpful documentation |
| 14:56.26 | brlcad | right at the top of the file, it notes that there's a convention in place with "make" functions that pair with "kill" functions ... |
| 14:56.47 | brlcad | so right then would be a hint that you probably need to call a kill function |
| 14:58.08 | brlcad | to confirm, you'd continue to the actual nmg_mm() definition where we fortunately do find it's documented with a comment |
| 14:59.42 | brlcad | alas, it doesn't say specficially that calling nmg_km() is expected (problem #2), so you're left with the prior convention comment to follow from the top of the file or reading the implementation |
| 15:00.38 | brlcad | reading the implementation, you'd see it calls NMG_GETSTRUCT() .. so presuambly either you need to call something else like maybe "NMG_PUTSTRUCT()" or call some other function that calls something like NMG_PUTSTRUCT() |
| 15:01.59 | brlcad | and following the convention, looking at nmg_km(), it looks like it does free memory so it's a safe assumption that that is the routine to call |
| 15:02.40 | brlcad | that is the through process you should follow for pretty much all code you are unfamiliar with |
| 15:03.00 | brlcad | you chase the rabbit down the rabbit hole until you understand, don't assume or make guesses |
| 15:04.27 | brlcad | Zhao_Anqing: questions? |
| 15:12.28 | Zhao_Anqing | brlcad: That' OK. I get it. I should be completely clear with the routines/codes which I deal with. tracking the routines and reading the comments. Is it right? |
| 15:18.02 | Zhao_Anqing | And, should the comment be located at *.h, not *.c? |
| 15:19.04 | Zhao_Anqing | like nmg_mm(), it's detailed comment is in nmg_mk.c, not in raytrace.h. Is it wrong? |
| 15:41.59 | starseeker | brlcad: it comes down to user expectation, I guess - if the most common uses of comb are in fact to alter existing geometry, then you don't want to default to "don't change anything" since it's just a lot of extra typing. |
| 15:43.23 | starseeker | The whole "-S" option was probably little more than an afterthought that I should have left out at that stage - my primary focus was on other options, should have kept it there :-/ |
| 16:08.55 | *** join/#brlcad FreezingCold (~FreezingC@135.0.41.14) | |
| 16:23.02 | *** join/#brlcad teepee (~teepee@gateway/tor-sasl/teepee) | |
| 16:32.01 | *** join/#brlcad teepee_ (~teepee@gateway/tor-sasl/teepee) | |
| 16:43.20 | *** join/#brlcad javampire (~ncsaba@p4FF72C63.dip0.t-ipconnect.de) | |
| 16:59.33 | *** join/#brlcad teepee (~teepee@gateway/tor-sasl/teepee) | |
| 17:00.10 | *** join/#brlcad javampire (~ncsaba@p4FF72C63.dip0.t-ipconnect.de) | |
| 17:00.54 | javampire | kanzure: Hi Bryan, thanks for merging the pull request ! |
| 17:01.05 | kanzure | okay |
| 17:01.47 | javampire | there was really nothing to comment on it ? I'm still learning python, so don't hold back if there's anything bad in my code :-) |
| 17:06.54 | brlcad | Zhao_Anqing: yes you should be tracking the routines and reading the comments (otherwise, you have no way of knowing/learning by yourself) |
| 17:07.12 | brlcad | and we do want all of the *public api* comments to be in the .h files |
| 17:10.12 | brlcad | Zhao_Anqing: also when you come across deficiencies (like problems #1 and #2 I mentioned), those are usually opportunities to improve the code (e.g., a patch/commit fix) |
| 17:13.25 | brlcad | starseeker: yes, user expectation is key |
| 17:14.08 | brlcad | "a lot of extra typing" is just 3-chars, but if that's nearly always used, it would be silly to have to specify it every time |
| 17:15.13 | brlcad | but then if it really is intended as an append/modify command, I'd be of mind to eliminate the -S option |
| 17:16.57 | brlcad | what I'm saying is that comb's current failing is that it's a create and/or append command and that's not deterministic looking at the command issue (you have no idea what their intention is by the command string alone) |
| 17:18.03 | brlcad | so we should make it be one or the other with an option to allow the other (e.g., append that fails if doesn't exist with flag to create OR create that fails if it does exist with flag to append) |
| 17:18.51 | Notify | 03BRL-CAD:brlcad * 60523 brlcad/trunk/NEWS: cliff Added the -bool option to search, which allows filtering based on whether a given instance of an object is combined into its parent comb with a union (u), intersection (+), or subtraction (-) boolean operator |
| 17:22.35 | Notify | 03BRL-CAD:brlcad * 60524 brlcad/trunk/TODO: comb command should be deterministic |
| 17:25.50 | *** join/#brlcad javampire_ (~ncsaba@p4FF72C63.dip0.t-ipconnect.de) | |
| 17:26.48 | Notify | 03BRL-CAD:brlcad * 60525 (brlcad/trunk/doc/docbook/articles/en/build_pattern.xml brlcad/trunk/doc/docbook/articles/en/build_region.xml and 302 others): <!-- Converted by db4-upgrade version 1.0 --> |
| 17:31.38 | Notify | 03BRL-CAD:brlcad * 60526 brlcad/trunk/src/libged/comb.c: avoid the _ged_ prefix, use noun+verb convention. these are HIDDEN. |
| 17:37.32 | Notify | 03BRL-CAD:brlcad * 60527 brlcad/trunk/NEWS: nick fixed a bug in archer (r56389) where editing something drawn in hidden line mode caused it to be redrawn in shaded mode. |
| 17:45.02 | Notify | 03BRL-CAD:brlcad * 60528 brlcad/trunk/src/libicv/encoding.c: ws |
| 17:45.41 | *** join/#brlcad javampire__ (~ncsaba@p4FF72C63.dip0.t-ipconnect.de) | |
| 17:48.27 | Notify | 03BRL-CAD:brlcad * 60529 brlcad/trunk/src/libicv/encoding.c: use lrint() to round with range validation, instead of manually rounding to nearest assuming positive small values. |
| 17:55.24 | Notify | 03BRL-CAD:brlcad * 60530 brlcad/trunk/src/libbrep/intersect.cpp: ws |
| 18:02.10 | *** join/#brlcad javampire (~ncsaba@p4FF72C63.dip0.t-ipconnect.de) | |
| 18:02.33 | Notify | 03BRL-CAD:brlcad * 60531 (brlcad/trunk/src/libbrep/PullbackCurve.cpp brlcad/trunk/src/libbrep/PullbackCurve.h): remove dead code: test1_pullback_curve and test2_pullback_curve |
| 18:02.41 | *** join/#brlcad javampire (~ncsaba@p4FF72C63.dip0.t-ipconnect.de) | |
| 18:04.56 | Notify | 03BRL-CAD:brlcad * 60532 brlcad/trunk/src/burst/burst.c: no new globals. the option string doesn't need to be static too. |
| 18:15.05 | Notify | 03BRL-CAD:brlcad * 60533 brlcad/trunk/doc/burst/run_doclifter.sh: debug.mm does not exist here, switch to burst |
| 18:15.19 | Notify | 03BRL-CAD:brlcad * 60534 brlcad/trunk/doc/burst/burst.mm: doclifter doesn't know what to do with .SM |
| 18:20.17 | Notify | 03BRL-CAD:brlcad * 60535 brlcad/trunk/doc/burst/Make-tables.sh: gnu tbl is gtbl-compatible |
| 18:27.47 | *** join/#brlcad javampire (~ncsaba@p4FF72C63.dip0.t-ipconnect.de) | |
| 18:31.40 | Notify | 03BRL-CAD:starseeker * 60536 brlcad/trunk/src/librt/primitives/nmg/nmg_misc.c: Guard against coords==0, but leave the note about the error - need to make sure there isn't some deeper nmg issue involved that might result in coords being zero... |
| 18:38.18 | Notify | 03BRL-CAD:brlcad * 60537 brlcad/trunk/doc/burst/title.mm: the old address is no longer relevant. |
| 18:47.28 | Notify | 03BRL-CAD:starseeker * 60538 brlcad/trunk/regress/CMakeLists.txt: Neither the repository test or flawfinder need rt |
| 19:04.33 | Notify | 03BRL-CAD:brlcad * 60539 brlcad/trunk/src/librt/primitives/nmg/nmg_misc.c: RT_NURB_EXTRACT_COORDS is returning the dimensionality of the nurbs curve points (e.g., 2d, 3d, 4d). for whatever reason it supports varying dimensionality (probably as a means to store both uv and 3d control points, or rational and irrational numbers, in the same container). don't just tack in another depth to the logic and leave the |
| 19:04.37 | Notify | comment turd. the dimensionality of points needs to be greater than zero for this loop to work at all, so just check that it's what is expected. |
| 19:19.14 | *** join/#brlcad teepee (~teepee@gateway/tor-sasl/teepee) | |
| 19:22.24 | *** join/#brlcad caen23 (~caen23@109.97.108.82) | |
| 19:37.31 | Notify | 03BRL-CAD:starseeker * 60540 brlcad/trunk/include/CMakeLists.txt: Make the check for public headers including private headers part of the CMake process. It's currently configure time (ideally it would be build time, but that's more compilcated) and will check the CMake lists of private and public headers as defined in include/CMakeLists.txt. Should make the repository.sh regression test obsolete, but needs testing |
| 19:37.32 | Notify | to be sure of that. |
| 19:40.18 | starseeker | it's returning the dimensionality? |
| 19:40.42 | starseeker | didn't get that from the name RT_NURB_EXTRACT_COORDS... |
| 19:43.43 | starseeker | reflects that r60540 will make ONE of the regression tests in repository.sh obsolete - comment wasn't clear enough |
| 19:45.12 | starseeker | is torn... to do that test right it really should be a build-time test... |
| 19:47.32 | *** join/#brlcad hcurtis (b82d3331@gateway/web/freenode/ip.184.45.51.49) | |
| 20:19.13 | brlcad | starseeker: yeah, I never go by the name ..that's just a hint :) |
| 20:19.38 | brlcad | all devs are terrible at naming things given enough time and enough code, self included ;) |
| 20:20.50 | brlcad | they usually get progressively better as a dev matures, and they see more patterns working with more code .. but not always! |
| 20:22.42 | brlcad | and yeah, I agree -- that should be a build-time test (associated with a build target like regress) |
| 20:23.20 | brlcad | we don't want to be all "yeah, someone committed something bad, so f-you. not gonna even let you try to build." |
| 20:23.24 | *** join/#brlcad mandy_ (~mandy@117.199.105.36) | |
| 20:24.38 | brlcad | what about just making that a macro that lives in regress/, then some build rule can call the macro? |
| 20:24.42 | brlcad | waves to mandy_ |
| 20:30.21 | *** join/#brlcad FreezingCold (~FreezingC@135.0.41.14) | |
| 21:00.07 | ``Erik | "There are only two hard things in Computer Science: cache invalidation, naming things, and off-by-1 errors." |
| 21:12.03 | starseeker | brlcad: I was thinking to make it like the astyle validation - let the compile proceed, but hault afterwards. |
| 21:12.41 | starseeker | brlcad: the idea was to enforce the key "HACKING" rules that can be so enforced in the same way we're looking to enforce style |
| 21:13.14 | starseeker | I'm pretty sure I can do it, given the astyle proof of concept |
| 21:22.41 | ``Erik | (so ci is back to jenkins, not buildbot? between jenkins, fisheye and bamboo, almost half of bz's working ram is eaten by jvms) |
| 21:23.17 | ``Erik | wonders if all three of those could be loaded into a single jvm working as like an app server or something |
| 21:46.00 | Notify | 03BRL-CAD:starseeker * 60541 brlcad/trunk/include/CMakeLists.txt: Comment this out for now - needs to be an astyle type build-time test. Logic will be the same, but needs to live in its own 'hacking_test_#.cmake.in' or some such. |
| 21:46.20 | *** join/#brlcad hcurtis_ (4ab29b06@gateway/web/freenode/ip.74.178.155.6) | |
| 21:46.24 | starseeker | is sorta leaning toward buildbot at this point... |
| 22:15.22 | *** join/#brlcad FreezingCold (~FreezingC@135.0.41.14) | |
| 22:40.25 | Zhao_Anqing | brlcad: OK. Thank you for detailed guidiance. I learn a good lesson and benifited from your words step by step. I am really appriciate for that. |
| 22:41.00 | Zhao_Anqing | I will remember this. |
| 22:41.52 | Zhao_Anqing | BTW, I went to bed and just wake up, so be delay to reply. |
| 22:42.03 | Zhao_Anqing | ^-^ |
| 23:51.06 | *** join/#brlcad hcurtis (b82d6ff9@gateway/web/freenode/ip.184.45.111.249) | |