On Tue, 2008-09-16 at 12:44 +0200, Georg Martius wrote: > > Yup, applied. Mostly OK, but there is still some code that should be > > updated. No hurry for that. > Okay thanks! > Sorry, I must have overlooked something. Also to be honest I checked other > parts of transcode and it is partly not very close to the style guide ;-)
Yes, we know ;) Before the 1.1.0 cycle, which has lead to formalization of the STYLE guide, transcode was a mess^Wpatchwork of various coding styles. Now we have guidelines, and the code is slowly but steadily being adeguated -or just rewritten- as the development goes on. Some parts are still untouched, however (most notably the greatest part of the import and filter layers). > One thing I was not sure about was whether function documentation goes in .h > and .c file or only in one place. I myself would put in .h ( at least for the > public interface) and the rest in .c. However I found in the other code > mostly the comment block in the .c file, so that's what i did. Duplicating > comment is in my eyes a bad idea because it will be outdated very quickly. The style guidelines are likely vague on this point. My own rule is the following: - for public functions/data structures, document them in the header file, and leave the implementation in the C source file naked. - for private functions/data structures, document them in the C source file. Anyway there aren't strict rules for that, even document everything in the C source file is accepted. > Okay, I am happy to change my code to a new API. My implementation copies the > entries, not to have unwanted side-effect and to handle cleanup easily. For > my purpose it is really no problem. In case you want to avoid the extra copy > cost I see suggest to provide two APIs : > 1) the easy interface (as it is) and > 2) one for handling memory at the caller side with some descriptive suffix > like linkedList_add_insitu(...) and linkedList_delete_withdata() (same > as > in 1), linkedList_delete_withoutdata() and so on My early idea, currently being prototyped, is to let the caller side handling all the memory allocation for data inside list elements; the list code should handle only the memory needed for it's own internal purposes. (one-line-rationale: don't force the caller to adopt any convention about memory handling, let the thing works for static data too [think to external indexes]) But I'll also add a few convenience functions/macro built on top of that that copies data. [...] > > The core idea is that one shouldn't be _forced_ to perform fake runs of > > transcode just to inspect the module characteristics: use tcmodinfo (as > > the executable name suggests!) separately instead. > > > Okay I understand that but I still think > a) it would not harm to print the parameter help when -J filtername=help is > used, and it should be no problem to implement at the pluginhandler side. yes it is not a problem but I still don't like it from a design point of view. Asking for help to a module, in the NMS, is the same thing as inspecting for a module properties. During transcode invocations, there should be no inspection, only settings. If an user want to inspect, it should use tcmodinfo instead. > b) if this will not be provided, then all the filter documentation has to be > changed to not mention the "help" option. I agree that this should be explained better, but I don't think remove it altogether is the best thing to do. > c) tcmodinfo should be documented for the user e.g. if transcode --help is > called. Yes indeed, this has to be done. > > Just embed them in the main source, there is no official guidelines for > > that, every module goes on it's own way. > So in principle I could keep my own list of Todos and stuff in the stabilize > filter directory? yes but it's better (even if not required) to just use comments in the source file instead of a separate file. > > PS: the cast after the malloc is useless and it should be removed > I am sorry I didn't know that. NP to remove that. Why is the typecast > useless, > malloc as return type (void*)?! Because the implicit C type conversions :) Not using the casts it is perfectly safe, and in some circumstances can be a little more safe indeed (see http://c-faq.com/malloc/cast.html - and related entries- for example) But it is *mostly* (but not exclusively) a matter of style Bests, -- Francesco Romani // Ikitt
