Compare commits

...

7 Commits

Author SHA1 Message Date
Jean-Marc Lasgouttes
375aba7b5d Transform CoordCache::check/checkDim in ASSERT_DIM/POS macros
This makes it more obvious to the reader that these are actually
assertions, and should help Coverity scan too.
2024-09-13 15:44:33 +02:00
Jean-Marc Lasgouttes
3af0bad22a Pass shared_ptr<> arguments by const reference
These arguments are not expensive to copy. However, it makes sense to
pass such pointers by const reference when they are just intended for
reading.

Some reading about this issue is here:
https://www.internalpointers.com/post/move-smart-pointers-and-out-functions-modern-c

Fixes some Coverity scan defects.
2024-09-13 14:54:43 +02:00
Jean-Marc Lasgouttes
58ee4c9ec7 Pass DocumentClassConstPtr arguments by const reference
This arguments are shared_ptr objects, so they are not expensive to
copy. However, it makes sense to pass such pointers by const reference
when they are just intended for reading.

Some reading about this issue is here:
https://www.internalpointers.com/post/move-smart-pointers-and-out-functions-modern-c

Fixes some Coverity scan defects.
2024-09-13 14:16:21 +02:00
Jean-Marc Lasgouttes
3649cea9e5 Make two parameters const references 2024-09-13 11:27:00 +02:00
Jean-Marc Lasgouttes
ffca6730d6 Fix uninitialized variable
Spotted by Coverity scan.
2024-09-13 11:21:19 +02:00
Jean-Marc Lasgouttes
9f40eaee15 Do not use rand() to set a BranchList id
Use a simple counting instead, beecause Coverity complains that rand()
is not safe, and counting is siimpler anyway.
2024-09-13 11:09:30 +02:00
Jean-Marc Lasgouttes
9b3c28178c Make string parameter a const reference
Spotted by Coverity scan.
2024-09-13 10:27:24 +02:00
23 changed files with 62 additions and 59 deletions

View File

@ -32,6 +32,11 @@ docstring const & Branch::branch() const
return branch_; return branch_;
} }
static int list_id_generator = 0;
BranchList::BranchList()
: separator_(from_ascii("|")), id_(++list_id_generator) {}
void Branch::setBranch(docstring const & s) void Branch::setBranch(docstring const & s)
{ {

View File

@ -14,7 +14,6 @@
#include "support/docstring.h" #include "support/docstring.h"
#include <cstdlib> // rand()
#include <list> #include <list>
@ -104,7 +103,7 @@ public:
typedef List::const_iterator const_iterator; typedef List::const_iterator const_iterator;
/// ///
BranchList() : separator_(from_ascii("|")), id_(rand()) {} BranchList();
/// ///
docstring separator() const { return separator_; } docstring separator() const { return separator_; }

View File

@ -2752,7 +2752,7 @@ DocumentClassConstPtr BufferParams::documentClassPtr() const
} }
void BufferParams::setDocumentClass(DocumentClassConstPtr tc) void BufferParams::setDocumentClass(DocumentClassConstPtr const & tc)
{ {
// evil, but this function is evil // evil, but this function is evil
doc_class_ = const_pointer_cast<DocumentClass>(tc); doc_class_ = const_pointer_cast<DocumentClass>(tc);

View File

@ -165,7 +165,7 @@ public:
/// This bypasses the baseClass and sets the textClass directly. /// This bypasses the baseClass and sets the textClass directly.
/// Should be called with care and would be better not being here, /// Should be called with care and would be better not being here,
/// but it seems to be needed by CutAndPaste::putClipboard(). /// but it seems to be needed by CutAndPaste::putClipboard().
void setDocumentClass(DocumentClassConstPtr); void setDocumentClass(DocumentClassConstPtr const &);
/// List of modules in use /// List of modules in use
LayoutModuleList const & getModules() const { return layout_modules_; } LayoutModuleList const & getModules() const { return layout_modules_; }
/// List of default modules the user has removed /// List of default modules the user has removed

View File

@ -1162,7 +1162,7 @@ void BufferView::makeDocumentClass()
} }
void BufferView::updateDocumentClass(DocumentClassConstPtr olddc) void BufferView::updateDocumentClass(DocumentClassConstPtr const & olddc)
{ {
StableDocIterator backcur(d->cursor_); StableDocIterator backcur(d->cursor_);
ErrorList & el = buffer_.errorList("Class Switch"); ErrorList & el = buffer_.errorList("Class Switch");

View File

@ -456,7 +456,7 @@ private:
void updateHoveredInset() const; void updateHoveredInset() const;
/// ///
void updateDocumentClass(DocumentClassConstPtr olddc); void updateDocumentClass(DocumentClassConstPtr const & olddc);
/// ///
int width_; int width_;
/// ///

View File

@ -11,9 +11,7 @@
#include "CoordCache.h" #include "CoordCache.h"
#include "support/debug.h" #include "support/debug.h"
#include "support/lassert.h" #include "support/lassert.h"

View File

@ -11,9 +11,6 @@
#ifndef COORDCACHE_H #ifndef COORDCACHE_H
#define COORDCACHE_H #define COORDCACHE_H
// It seems that MacOSX define the check macro.
#undef check
#include "Dimension.h" #include "Dimension.h"
#include <unordered_map> #include <unordered_map>
@ -23,6 +20,24 @@ namespace lyx {
class Inset; class Inset;
class MathData; class MathData;
#ifdef ENABLE_ASSERTIONS
#define ASSERT_HAS_DIM(thing, hint) \
if (data_.find(thing) != data_.end()) \
{} \
else \
lyxbreaker(thing, hint, data_.size());
#define ASSERT_HAS_POS(thing, hint) \
auto it = data_.find(thing); \
if (it != data_.end() && it->second.pos.x != -10000) \
{} \
else \
lyxbreaker(thing, hint, data_.size());
#else
#define ASSERT_HAS_DIM(thing, hint)
#define ASSERT_HAS_POS(thing, hint)
#endif
void lyxbreaker(void const * data, const char * hint, int size); void lyxbreaker(void const * data, const char * hint, int size);
struct Geometry { struct Geometry {
@ -94,37 +109,37 @@ public:
Geometry & geometry(T const * thing) Geometry & geometry(T const * thing)
{ {
checkDim(thing, "geometry"); ASSERT_HAS_DIM(thing, "geometry");
return data_.find(thing)->second; return data_.find(thing)->second;
} }
Geometry const & geometry(T const * thing) const Geometry const & geometry(T const * thing) const
{ {
checkDim(thing, "geometry"); ASSERT_HAS_DIM(thing, "geometry");
return data_.find(thing)->second; return data_.find(thing)->second;
} }
Dimension const & dim(T const * thing) const Dimension const & dim(T const * thing) const
{ {
checkDim(thing, "dim"); ASSERT_HAS_DIM(thing, "dim");
return data_.find(thing)->second.dim; return data_.find(thing)->second.dim;
} }
int x(T const * thing) const int x(T const * thing) const
{ {
check(thing, "x"); ASSERT_HAS_POS(thing, "x");
return data_.find(thing)->second.pos.x; return data_.find(thing)->second.pos.x;
} }
int y(T const * thing) const int y(T const * thing) const
{ {
check(thing, "y"); ASSERT_HAS_POS(thing, "y");
return data_.find(thing)->second.pos.y; return data_.find(thing)->second.pos.y;
} }
Point xy(T const * thing) const Point xy(T const * thing) const
{ {
check(thing, "xy"); ASSERT_HAS_POS(thing, "xy");
return data_.find(thing)->second.pos; return data_.find(thing)->second.pos;
} }
@ -159,24 +174,6 @@ public:
private: private:
friend class CoordCache; friend class CoordCache;
#ifdef ENABLE_ASSERTIONS
void checkDim(T const * thing, char const * hint) const
{
if (!hasDim(thing))
lyxbreaker(thing, hint, data_.size());
}
void check(T const * thing, char const * hint) const
{
if (!has(thing))
lyxbreaker(thing, hint, data_.size());
}
#else
void checkDim(T const *, char const * const ) const {}
void check(T const *, char const *) const {}
#endif
typedef std::unordered_map<T const *, Geometry> cache_type; typedef std::unordered_map<T const *, Geometry> cache_type;
cache_type data_; cache_type data_;
}; };
@ -213,12 +210,16 @@ public:
/// Dump the contents of the cache to lyxerr in debugging form /// Dump the contents of the cache to lyxerr in debugging form
void dump() const; void dump() const;
private: private:
/// MathDatas /// MathDatas
Arrays arrays_; Arrays arrays_;
// All insets // All insets
Insets insets_; Insets insets_;
}; };
#undef ASSERT_HAS_DIM
#undef ASSERT_HAS_POS
} // namespace lyx } // namespace lyx
#endif #endif

View File

@ -600,7 +600,7 @@ PitPosPair eraseSelectionHelper(BufferParams const & params,
} }
Buffer * copyToTempBuffer(ParagraphList const & paragraphs, DocumentClassConstPtr docclass) Buffer * copyToTempBuffer(ParagraphList const & paragraphs, DocumentClassConstPtr const & docclass)
{ {
// This used to need to be static to avoid a memory leak. It no longer needs // This used to need to be static to avoid a memory leak. It no longer needs
// to be so, but the alternative is to construct a new one of these (with a // to be so, but the alternative is to construct a new one of these (with a
@ -828,15 +828,15 @@ bool multipleCellsSelected(CursorData const & cur)
} }
void switchBetweenClasses(DocumentClassConstPtr oldone, void switchBetweenClasses(DocumentClassConstPtr const & oldone,
DocumentClassConstPtr newone, InsetText & in) { DocumentClassConstPtr const & newone, InsetText & in) {
ErrorList el = {}; ErrorList el = {};
switchBetweenClasses(oldone, newone, in, el); switchBetweenClasses(oldone, newone, in, el);
} }
void switchBetweenClasses(DocumentClassConstPtr oldone, void switchBetweenClasses(DocumentClassConstPtr const & oldone,
DocumentClassConstPtr newone, InsetText & in, ErrorList & errorlist) DocumentClassConstPtr const & newone, InsetText & in, ErrorList & errorlist)
{ {
errorlist.clear(); errorlist.clear();

View File

@ -138,11 +138,11 @@ void pasteParagraphList(Cursor & cur, ParagraphList const & parlist,
* It changes layouts and character styles. Errors are reported * It changes layouts and character styles. Errors are reported
* in the passed ErrorList. * in the passed ErrorList.
*/ */
void switchBetweenClasses(DocumentClassConstPtr oldone, void switchBetweenClasses(DocumentClassConstPtr const & oldone,
DocumentClassConstPtr newone, InsetText & in, ErrorList & el); DocumentClassConstPtr const & newone, InsetText & in, ErrorList & el);
/// Same but without error reporting. /// Same but without error reporting.
void switchBetweenClasses(DocumentClassConstPtr oldone, void switchBetweenClasses(DocumentClassConstPtr const & oldone,
DocumentClassConstPtr newone, InsetText & in); DocumentClassConstPtr const & newone, InsetText & in);
/// Get the current selection as a string. Does not change the selection. /// Get the current selection as a string. Does not change the selection.
/// Does only work if the whole selection is in mathed. /// Does only work if the whole selection is in mathed.

View File

@ -24,7 +24,7 @@ namespace lyx {
TocBuilder::TocBuilder(shared_ptr<Toc> toc) TocBuilder::TocBuilder(shared_ptr<Toc> const & toc)
: toc_(toc ? toc : make_shared<Toc>()), : toc_(toc ? toc : make_shared<Toc>()),
stack_() stack_()
{ {

View File

@ -27,7 +27,7 @@ class DocIterator;
class TocBuilder class TocBuilder
{ {
public: public:
TocBuilder(std::shared_ptr<Toc> toc); TocBuilder(std::shared_ptr<Toc> const & toc);
/// Open a level. /// Open a level.
/// When entering a float or flex or paragraph (with AddToToc) /// When entering a float or flex or paragraph (with AddToToc)
void pushItem(DocIterator const & dit, docstring const & s, void pushItem(DocIterator const & dit, docstring const & s,

View File

@ -40,7 +40,7 @@ Action::Action(FuncRequest func, QIcon const & icon, QString const & text,
} }
Action::Action(shared_ptr<FuncRequest const> func, Action::Action(shared_ptr<FuncRequest const> const & func,
QIcon const & icon, QString const & text, QIcon const & icon, QString const & text,
QString const & tooltip, QObject * parent) QString const & tooltip, QObject * parent)
: QAction(parent), func_(func), icon_(icon) : QAction(parent), func_(func), icon_(icon)

View File

@ -39,7 +39,7 @@ public:
// Takes shared ownership of func. // Takes shared ownership of func.
// Use for perf-sensitive code such as populating menus. // Use for perf-sensitive code such as populating menus.
Action(std::shared_ptr<FuncRequest const> func, Action(std::shared_ptr<FuncRequest const> const & func,
QIcon const & icon, QString const & text, QIcon const & icon, QString const & text,
QString const & tooltip, QObject * parent); QString const & tooltip, QObject * parent);

View File

@ -82,7 +82,7 @@ static docstring fix_name(string const & str, bool big)
} }
struct MathSymbol { struct MathSymbol {
MathSymbol(char_type uc = '?', string icon = string()) MathSymbol(char_type uc = '?', string const & icon = string())
: unicode(uc), icon(icon) : unicode(uc), icon(icon)
{} {}
char_type unicode; char_type unicode;

View File

@ -149,7 +149,7 @@ struct GuiWorkArea::Private
QTransform item_trans_; QTransform item_trans_;
bool item_trans_needs_reset_ = false; bool item_trans_needs_reset_ = false;
/// for debug /// for debug
QLocale::Language im_lang_; QLocale::Language im_lang_ = QLocale::AnyLanguage;
/// Ratio between physical pixels and device-independent pixels /// Ratio between physical pixels and device-independent pixels
/// We save the last used value to detect changes of the /// We save the last used value to detect changes of the

View File

@ -160,7 +160,7 @@ void TocModel::updateItem(DocIterator const & dit)
} }
void TocModel::reset(shared_ptr<Toc const> toc) void TocModel::reset(shared_ptr<Toc const> const & toc)
{ {
toc_ = toc; toc_ = toc;
if (toc_->empty()) { if (toc_->empty()) {

View File

@ -39,7 +39,7 @@ public:
/// ///
TocModel(QObject * parent); TocModel(QObject * parent);
/// ///
void reset(std::shared_ptr<Toc const>); void reset(std::shared_ptr<Toc const> const &);
/// ///
void reset(); void reset();
/// ///

View File

@ -258,7 +258,7 @@ bool InsetIndexMacro::getStatus(Cursor & cur, FuncRequest const & cmd,
void InsetIndexMacro::processLatexSorting(otexstream & os, OutputParams const & runparams, void InsetIndexMacro::processLatexSorting(otexstream & os, OutputParams const & runparams,
docstring const latex, docstring const plain) const docstring const & latex, docstring const & plain) const
{ {
if (contains(latex, '\\') && !contains(latex, '@')) { if (contains(latex, '\\') && !contains(latex, '@')) {
// Plaintext might return nothing (e.g. for ERTs). // Plaintext might return nothing (e.g. for ERTs).

View File

@ -101,7 +101,7 @@ private:
docstring toolTip(BufferView const & bv, int x, int y) const override; docstring toolTip(BufferView const & bv, int x, int y) const override;
/// ///
void processLatexSorting(otexstream &, OutputParams const &, void processLatexSorting(otexstream &, OutputParams const &,
docstring const, docstring const) const; docstring const &, docstring const &) const;
/// ///
bool hasSortKey() const; bool hasSortKey() const;
/// ///

View File

@ -158,7 +158,7 @@ void FileMonitorGuard::notifyChange(QString const & path)
} }
FileMonitor::FileMonitor(std::shared_ptr<FileMonitorGuard> monitor) FileMonitor::FileMonitor(std::shared_ptr<FileMonitorGuard> const & monitor)
: monitor_(monitor) : monitor_(monitor)
{ {
connectToFileMonitorGuard(); connectToFileMonitorGuard();
@ -193,7 +193,7 @@ void FileMonitor::changed(bool const exists)
} }
ActiveFileMonitor::ActiveFileMonitor(std::shared_ptr<FileMonitorGuard> monitor, ActiveFileMonitor::ActiveFileMonitor(std::shared_ptr<FileMonitorGuard> const & monitor,
FileName const & filename, int interval) FileName const & filename, int interval)
: FileMonitor(monitor), filename_(filename), interval_(interval), : FileMonitor(monitor), filename_(filename), interval_(interval),
timestamp_(0), checksum_(0), cooldown_(true) timestamp_(0), checksum_(0), cooldown_(true)

View File

@ -128,7 +128,7 @@ class FileMonitor : public QObject
Q_OBJECT Q_OBJECT
public: public:
FileMonitor(std::shared_ptr<FileMonitorGuard> monitor); FileMonitor(std::shared_ptr<FileMonitorGuard> const & monitor);
typedef signal<void(bool)> sig; typedef signal<void(bool)> sig;
typedef sig::slot_type slot; typedef sig::slot_type slot;
@ -166,7 +166,7 @@ class ActiveFileMonitor : public FileMonitor
{ {
Q_OBJECT Q_OBJECT
public: public:
ActiveFileMonitor(std::shared_ptr<FileMonitorGuard> monitor, ActiveFileMonitor(std::shared_ptr<FileMonitorGuard> const & monitor,
FileName const & filename, int interval); FileName const & filename, int interval);
/// call checkModified asynchronously /// call checkModified asynchronously
void checkModifiedAsync(); void checkModifiedAsync();