We probably should add a note to the coverity error and change the status to not a bug so that this doesn't come up again in the future. There are a few coverity errors where the code is written as intended but coverity doesn't know that and flags them as errors.
On 06/17/2018 08:33 AM, Maciej Suminski wrote: > Hi Simon, > > The code in question has been inspired by LIB_PART::CopySelectedItems() > which deselects fields. There the original items need to be deselected, > and the copied ones are kept selected and dragged to a new location. > > Method LIB_EDIT_FRAME::copySelectedItems() is used in two actions: copy > and cut. It does not matter whether fields are kept selected when > copying, as selection flag is cleared anyway (block_libedit.cpp:162). > The selection flag seems to matter in cut operation, as it deletes > selected items (block_libedit.cpp:169) after creating a copy. > > Looking further, LIB_PART::DeleteSelectedItems() also clears selection > flag for fields, so effectively fields deselection in > copySelectedItems() is just a redundant protection. > > Is there any particular issue caused by the code? Do not hesitate to ask > questions regarding the code when in doubts, but IMHO the bug tracker > should be left for reporting problems. > > Regards, > Orson > > On 06/16/2018 11:20 PM, Simon Richter wrote: >> Hi, >> >> I'm doing a bit of refactoring, and keep stumbling over weird bits that >> may be historical, fix an actual problem or both, but where it isn't >> immediately obvious what is happening. >> >> For example, the LIB_EDIT_FRAME::copySelectedItems() deselects fields in >> order to not copy them -- simply skipping might also work, but there is >> probably a specific reason for that. Does this have any effect on other >> functions (e.g. if I select a bunch of items, start a copy and abort it, >> then the fields are no longer selected, but are they also redrawn?) >> >> All of these are minor issues, but they make things a bit unpredictable >> at times. Would it make sense to create bug reports for them, possibly >> tagged "wtf" so people who feel like refactoring some more find >> worthwhile targets? >> >> Simon >> >> >> >> _______________________________________________ >> Mailing list: https://launchpad.net/~kicad-developers >> Post to : kicad-developers@lists.launchpad.net >> Unsubscribe : https://launchpad.net/~kicad-developers >> More help : https://help.launchpad.net/ListHelp >> > > _______________________________________________ > Mailing list: https://launchpad.net/~kicad-developers > Post to : kicad-developers@lists.launchpad.net > Unsubscribe : https://launchpad.net/~kicad-developers > More help : https://help.launchpad.net/ListHelp > _______________________________________________ Mailing list: https://launchpad.net/~kicad-developers Post to : kicad-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~kicad-developers More help : https://help.launchpad.net/ListHelp
