Comment 6 for bug 1829562

Revision history for this message
Thomas Ward (teward) wrote :

FOR THE RECORD: I've been taking my time working on this review for about a week now, so as not to overwork vs. the other work I do outside Ubuntu.

Static Code Analysis Tools Results (CodeWarrior)

Static code analysis of the C/C++ driven files present shows some TOCTOU risk and potential command injection but the command injection doesn't *appear* to be actually command injection in the manner it suggests (therefore code injection is a false-positive match). TOCTOU risks are potentially present in how it handles "Time of check" vs. "Time of use" of searched files, but that's an upstream issue and as it has other mechanisms to handle such errors written into the code around the potential TOCTOU violations, I don't believe there's a major code issue blocking inclusion of the plugins.

------

Packaging Review:

* Copyright file is mostly complete, but it seems there are some redundant overlaps, possibly because of multiple licenses being used.

* Rules file looks clean.

* Install files look clean.

* debian/control specifically with package descriptors of built binaries needs some work, in my opinion:

 - ALL packages share the same description. This includes the metapackage and -common, and may lead to some confusion if someone is not in the know about what each package specifically provides (and if they are only reading package descriptions). Therefore, the short AND long descriptions could use some tweaking to provide a simple one-line description in the description about the specifics of what each package does. This may be me being overly specific, but adding that extra little bit into the description would go a long way to making this more acceptable. Specifics below.

 - As dsp-plugins-{lv2, ladspa, etc.} all provide *specific formats* of the packaged plugins, it is my belief each of those packages should in turn include a description that they contain those specific packaging formats of those packages for use in their corresponding compatible software that can use those formats of the same plugins. This includes in the short description (you could add " (LV2 format)" to the end of the -lv2 package for example.

 - The -common package should contain something to indicate it contains files common across *all* the varying packaged formats of the plugins.

 - The metapackage does not detail that it pulls in all packaged flavors of the same plugins. A simple description stating this about the metapackage would be preferable,

While it may not be directly blocking sponsorship regarding the lacking of these slightly extra descriptive lines in the package descriptions, I would prefer to see them in the specific 'packaging' formats of the plugins packages and the common package.