Apache OpenOffice (AOO) Bugzilla – Full Text Issue Listing |
Summary: | OpenOffice++ | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | porting | Reporter: | Daniel Darabos <darabos.daniel> | ||||||||||
Component: | code | Assignee: | AOO issues mailing list <issues> | ||||||||||
Status: | CONFIRMED --- | QA Contact: | |||||||||||
Severity: | Trivial | ||||||||||||
Priority: | P3 | CC: | issues, kami911, khirano, maho.nakata, Mathias_Bauer, nesshof, nikolai.pretzell, stephan.bergmann.secondary, thb, timar74 | ||||||||||
Version: | OOo 2.0 | ||||||||||||
Target Milestone: | not determined | ||||||||||||
Hardware: | All | ||||||||||||
OS: | All | ||||||||||||
URL: | http://oopp.multiracio.com/ | ||||||||||||
Issue Type: | TASK | Latest Confirmation in: | --- | ||||||||||
Developer Difficulty: | --- | ||||||||||||
Issue Depends on: | 77446, 77437, 77438, 77439, 77440, 77441, 77442, 77443, 77444, 77445, 77447 | ||||||||||||
Issue Blocks: | |||||||||||||
Attachments: |
|
Description
Daniel Darabos
2007-01-14 21:24:45 UTC
Created attachment 42152 [details]
Patches from Phase 1 (against 1.1.5)
Created attachment 42153 [details]
Patches from Phase 2 (against 2.0.4)
I'm tentatively changing the component to "porting", because "hu" was really not a very good choice (the patches have nothing to do with any language but C++). Sorry. I forgot to reassign the issue to the owner of porting (I guess that's Martin, right?). Sorry for fiddling so much with the issue... Looking at diffs.2.zip: 1 Evaluating the patches would be easier with some short information why these changes are considered necessary. 2 Since OOo 2.1, most C/C++ code is warning free and warnings are treated as errors (although unfortunately this is not the default behavior and has to be enabled with --enable-werror). I assume that some of your patches address issues that have already been solved when making the code warning free. GEN1007.diff: I assume these changes are necessary so that an object member is not used before it is initialized (at least the last two diffs are obviously of that kind, I did not check the others). This is a good change, yes. However, in some cases initializer lists in constructors will also have to be changed, to avoid warnings (= errors); for example, moving up m_aScrollSB in hangulhanjadlg.hxx needs to be reflected in the corresponding hangulhanjadlg.cxx. GEN7052.diff: I assume these changes are necessary to calculate double = int op double instead of double = int op int which are probably good changes (somebody knowing those code places should have a look, in case the truncation *was* intended). GEN7053.diff: For one, most of those places are probably better fixed by simply dropping the L suffix. For another, please do not introduce any new C style casts in C++ code. And in the patch to binfilter/bf_forms/source/component/forms_imgprod.cxx various lines lack closing parentheses, which looks very suspicious. thank you for the patches. To make review possible in a short time frame it would help if the patches are filed on per projects basis (Impress, Calc, Writer, framework, etc.) and attach a short description of the patches being made. Sorry, I was in a bit of a hurry yesterday and forgot to include the descriptions of the individual types of patches. So here are the definitions as reported by Colombus, the source code analyzer of the University of Szeged: GEN1007: data member is being initialized by a non-initialized data member (data members in the constructor initializer list are being initialized in the same order as they were declared in the class body) GEN7052: in case of storing the result of a division in a float/double variable, the operands should be casted to float/double to avoid rounding errors GEN7053: long integer literals should be casted to sal_Int32 when participating in an expression whose result's type is sal_Int32 I'm from the Multiracio Ltd. side of the project, so I'll try to get someone from Szeged here to answer questions about these issues. In particular I had my reservations about the casts in GEN7053, and that patch was created in the last minute -- this can be seen as the cause of the missing parentheses. I'll be attaching a fixed patched in a moment. The types of rule violations found in Phase 1 (in 1.1.5) are the following: GEN1004: classes that use dynamically allocated memory should have a copy constructor, an assignment operator and a destructor GEN1020: data members should be initialized GEN7035: switch instructions of integer type should have a 'default' label GEN7051: static variables should be declared as const We would of course like to recreate these patches for 2.1 (and seeing what got fixed already would be valuable feedback), and we would also like to be able to create an online source code monitoring system for OpenOffice.org (like the one currently available for Mozilla at http://frontendart.com/momo.php). I'm not sure about the organization of the project, but let's hope that we can secure the resources to do these. Thank you for looking at our patches. I will be reorganizing them by component affected and refiling them during the week. Created attachment 42169 [details]
fixed patch for Phase 2
added myself to cc list Created attachment 45158 [details]
Patches in unified format updated for 2.2
Hi, sorry about taking so long! We have now updated the patches for OpenOffice.org 2.2 (it was a pleasant surprise to see that a number of the patches became invalid because they got fixed!) and I have cleaned up their formatting. I have also split this file to small files by code module and I am going to file them as separate issues now so that they get to where they belong. Thank you for your patience and I hope we can help better OOo. We have also received some very valid and useful comments regarding these patches from Nikolai Pretzell, and we are (the guys at University Szeged are) preparing a new analysis based on the lessons learned. I have created the project specific issues. My apologies for spamming :). so patches are dispatched to subissues, reseting this issue to task for tracking the subtasks. Reset assigne to the default "issues@openoffice.apache.org". |