| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
The code does not rely on any new features, but we want
to ensure that it builds in those modes by default when
available.
Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
|
|
This fixes:
DLL_interface.cpp:187:19: error: passing an object that undergoes default
argument promotion to 'va_start'
has undefined behavior [-Werror,-Wvarargs]
va_start( args, aParamInfo ); // the element before the open param list
The fix works because bool can be converted to int (so old code still
works) while not being promoted (thus avoiding the undefined behavior).
Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
|
|
Building with recent Clang in C++ mode fails when using the non-standard
typeof operator. We can't rely on the new(ish) decltype yet, so use
the Boost implementation instead.
Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
|
|
Clang 4.8 started warning about unused static variables. ASCIIfyTable is
indeed only used under an ifdef. That can be resolved by moving the table next
to the code.
But Ansi_80_to_9F_to_UCS4 is used, and removing it leads to an undefined
symbol error. This seems to be a false positive. The only way to suppress
it is to add an external declaration.
Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
|
|
The latest release removed icaltzutil_set_exact_vtimezones_support.
It now always returns VTIMEZONE with multiple RRULEs, if necessary.
Therefore we need to use a different method to detect the modern
icalarray definition, and the usage of
icaltzutil_set_exact_vtimezones_support() itself must be limited to
just v2.
Original author: Milan Crha <mcrha@redhat.com>
Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
|
|
More recent cppcheck warns about uninitialized members and vars.
In some cases that's correct (but harmless?), in others it isn't.
Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
|
|
Several error paths returned an error code when a success boolean
should have been returned. Found with clang in C++11 mode.
Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
|
|
Static code analysis during compilation with clang found that an array
was freed with a plain delete. Code not used in practice by
SyncEvolution.
Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
|
|
"len" is unsigned, so subtracting 2 yields a very high value and then
the comparison against n causes memory to be read beyond the end of
the buffer when the buffer is smaller than 2.
Happens in SyncEvolution when sending a SAN message to a phone via
OBEX. In practice this typically had no effect because the
uninitialized memory is unlikely to contain exactly the sequence of
bytes that was checked for.
==28571== Invalid read of size 1
==28571== at 0x1275989: xltEncPcdata (xltenc.c:1177)
==28571== by 0x1274DAE: xltEncBlock (xltenc.c:1108)
==28571== by 0x1272DCD: xltEncBlock (xltenc.c:704)
==28571== by 0x1272698: xltEncInit (xltenc.c:332)
==28571== by 0x125ED1B: smlStartMessageExt (mgrcmdbuilder.c:207)
==28571== by 0x112066B: sysync::TSyncSession::StartMessage(sml_sync_hdr_s*) (syncsession.cpp:5512)
==28571== by 0x113F4A6: sysync::TEngineSessionDispatch::StartMessage(void*, void*, sml_sync_hdr_s*) (enginesessiondispatch.cpp:125)
==28571== by 0x11038DC: sysync::smlStartMessageCallback(void*, void*, sml_sync_hdr_s*) (syncappbase.cpp:2394)
==28571== by 0x125F7C7: mgrProcessStartMessage (mgrcmddispatcher.c:247)
==28571== by 0x125F552: smlProcessData (mgrcmddispatcher.c:138)
==28571== by 0x12200EA: sysync::TSyncAgent::ServerProcessingStep(unsigned short&, sysync::TEngineProgressType*) (syncagent.cpp:3289)
==28571== by 0x121FE2B: sysync::TSyncAgent::ServerSessionStep(unsigned short&, sysync::TEngineProgressType*) (syncagent.cpp:3257)
==28571== by 0x121F5FE: sysync::TSyncAgent::SessionStep(unsigned short&, sysync::TEngineProgressType*) (syncagent.cpp:3058)
==28571== by 0x114049D: sysync::TServerEngineInterface::SessionStep(sysync::SessionType*, unsigned short&, sysync::TEngineProgressType*) (enginesessiondispatch.cpp:478)
==28571== by 0x10DD6FC: sysync::SessionStep(void*, sysync::SessionType*, unsigned short*, sysync::TEngineProgressType*) (engineentry.cpp:88)
==28571== by 0x10D234F: sysync::TEngineModuleBridge::SessionStep(sysync::SessionType*, unsigned short&, sysync::TEngineProgressType*) (enginemodulebridge.cpp:109)
==28571== by 0x10CABCC: SyncEvo::SharedEngine::SessionStep(boost::shared_ptr<sysync::SessionType> const&, unsigned short&, sysync::TEngineProgressType*) (SynthesisEngine.cpp:94)
==28571== by 0xFDB098: SyncEvo::SyncContext::doSync() (SyncContext.cpp:4101)
==28571== by 0xFD6DDE: SyncEvo::SyncContext::sync(SyncEvo::SyncReport*) (SyncContext.cpp:3445)
==28571== by 0xE4FD57: SyncEvo::Cmdline::run() (Cmdline.cpp:1726)
==28571== by 0xC26689: main (syncevolution.cpp:531)
==28571== Address 0x13733782 is 0 bytes after a block of size 2 alloc'd
==28571== at 0x4C2BBAF: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==28571== by 0x126A2D0: wbxmlOpaqueToken (xltdecwbxml.c:680)
==28571== by 0x1269485: _nextTok (xltdecwbxml.c:341)
==28571== by 0x1264BAB: nextToken (xltdec.c:649)
==28571== by 0x126572C: buildPCData (xltdec.c:2368)
==28571== by 0x1264F60: buildSyncHdr (xltdec.c:777)
==28571== by 0x1264A75: xltDecInit (xltdec.c:474)
==28571== by 0x125F6FC: mgrProcessStartMessage (mgrcmddispatcher.c:225)
==28571== by 0x125F552: smlProcessData (mgrcmddispatcher.c:138)
==28571== by 0x12200EA: sysync::TSyncAgent::ServerProcessingStep(unsigned short&, sysync::TEngineProgressType*) (syncagent.cpp:3289)
...
Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
|
|
The Nokia N9 vCalendar implementation sends ==0A=0D= at the end of the
line when it should send just the =. Apparently the CRLF octets get
inserted before quoted-printable encoding and then get encoded.
Without working around that in our parser, we end up inserting extra
characters.
Tested and improved by deloptes@gmail.com.
Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
|
|
A newer cppcheck now also warns about syncop. This is a known
false positive because it is okay to store an uninitialized variable
in this case.
|
|
In (probably mostly unused code?) the previous boolean result
of getIncomingState() was compared against the enum, which looks
wrong. Probably getIncomingState() was meant to return the enum.
gcc6 warned about this:
synccommand.cpp:4277:36: error: comparison of constant '1' with boolean
expression is always false [-Werror=bool-compare]
fSessionP->getIncomingState()>psta_init, // alerted only if init complete
|
|
Recent gcc and/or C++14 changed the definition of abs() such that
applying it to a 16bit integer resulted in a float, leading to:
itemfield.cpp:1491:27: error: invalid operands of types
'__gnu_cxx::__enable_if<true, double>::__type {aka double}' and 'const int' to
binary 'operator%'
(uInt16)(abs(moffs) % MinsPerHour)
|
|
This fixes:
# warning "_BSD_SOURCE and _SVID_SOURCE are deprecated, use _DEFAULT_SOURCE"
|
|
libical v2 introduces a new API for generating system time zones
in a more interoperable format, with rules for summer and winter time (same
as in libical before v1) instead of just the transition dates (as in
libical v1). We should use that whenever possible.
We can drop support for libical embedded in libecal, that's really ancient.
Binary compatibility is now enabled with a dedicated configure parameter
(cleaner that piggy-backing on the parameter used by SyncEvolution). To make
this work for the libical v1/v2 ABI break (icalarray changed its types),
we must wrap some calls.
|
|
The source code was intentionally formatted by the upstream
authors so that statements sometimes align with surrounding
code even if indention according to program flow would be
different.
Instead of fixing large amounts of code, better just ignore
this warning.
|
|
Despite what the comment for the method said, the parameter was dereferenced
unconditionally and thus was not allowed to be NULL. Fixing the comment and
removing the redundant NULL check. Found with clang.
|
|
va_start() may allocate resources and thus has to be matched by
va_end(). Found with cppcheck 1.67.
|
|
va_start() may allocate resources and thus has to be matched by
va_end(). Found with cppcheck 1.67.
|
|
clang 3.5.0 warns about checking the "this" pointer because it cannot
be NULL. This safe-guard against incorrect usage of the class indeed
seems to be impossible, so remove the check.
|
|
Implicit conversion of long to int when calling abs() triggers a
compiler warning in clang 3.5.0.
|
|
The / in TZID values like Europe/Paris caused the code from "MIME
parser + encoder: no backslash quoting in parameter values" (8a7356c,
libsynthesis_3.4.0.47+syncevolution-1-4-99-2~6) to quote the
TZID value.
The Funambol OneMedia iCalendar 2.0 parser fails to handle this;
instead it imports events without applying the time zone.
It is not necessary to use quotation marks around values containing a
slash, so add the slash to the white list of characters which do no
trigger quoting.
|
|
|
|
This stricter version of RELAXEDCOMPARE() reports a difference also
when only the order of entries differs. This is easier to implement,
faster at runtime (min(n,m) comparisons instead of (n*m)), and takes
into account that the order may be relevant.
|
|
Caching mode in SyncEvolution only worked if the incoming item matched
the stored one exactly. Otherwise changing the order of items
(e.g. TEL followed by EMAIL instead of EMAIL followed by TEL) caused
arrays to be different (because of the shared LABEL array), and that
difference in the arrays led to rewriting items even if nothing
changed.
Because of the decode/encode step before storing a new item, caching
only worked reliably when the incoming item was also decoded and
encoded once, which is unnecessary overhead and only worked when using
a storage with the same encoding as the local sync.
A better solution is to use a more intelligent merge script:
- check whether arrays are identical except for ordering (done
with the new RELAXEDCOMPARE())
- ignore arrays which were found to be similar enough (by
passing a new parameter listing these fields to MERGEFIELDS().
In essence, MERGEFIELDS() then is only asked to check the remaining
fields or merge arrays if relevant differences were found.
At the moment, RELAXEDCOMPARE() only checks that entries in the arrays
match. It does not check that their order is the same. This could
(should!) be added and would also make the comparison faster when
there are differences (because it only needs to compare the next
non-empty entries).
|
|
and bug fixes
|
|
In one particular case, a pending command that needs more data, the engine
crashed because delayed command execution code path deleted the command still
needed for the next chunk (found in Client::Sync::eds_contact_eds_memo::testLargeObject).
|
|
SyncEvolution will intercept alloc/free during local sync to put
message buffers into shared memory and thus needs this feature
enabled.
|
|
This additional indirection allows users of the library to replace the
default implementation from libc.
The feature is optional. In libsynthesis it can be enabled in
target_options.h. Users of libsynthesis do not have access to
libmem.h, so they need to provide their own declaration of the
function pointers.
|
|
A signed long for the memory functions is inconsistent with
systems that have a standard malloc/realloc. Better use size_t.
|
|
Some code only compiles cleanly when MemSize_t is signed. Otherwise there are
signed/unsigned comparison and printf format warnings. Avoid those by casting
the values which are known to be positive to MemSize_t or long in debug
output.
|
|
Some functions were defined in both sml.h and libmem.h. Let's
use libmem.h as the authoritative place and eliminate the copies
in sml.h. Will be necessary when enhancing libmem.h.
|
|
The goal is to leave binfile completely untouched, including its "last
modified" time stamp, if nothing changes.
There are cases where the upper layers will call
TBinFileBase::updateRecord() and write the same data that already
is in the file. Instead of catching those case-by-case, let's check
at the lowest level.
On btrfs (and possibly other file systems), ftruncate() bumps the
"last modified" time stamp of a file even if the requested size
matches the existing one. Avoid that by skipping the ftruncate() when
the file already has the desired size.
|
|
During a normal sync where nothing changed, only the header gets
updated. This change is not critical and thus does not have to be
flushed to disk unless also some entries get added or updated.
The advantage is that when SyncEvolution detects a sync where nothing
changed on either side and skips the client's session shutdown, the
.bfi is left unchanged, which reduces flash wearout.
To detect item changes, a brute-force byte comparison is used. This
requires less changes to the code and is less error-prone than adding
"modified=true" to all places where "existingentries" gets modified.
|
|
When the configure auth type needs no nonce, don't generate one
and return NULL immediately, like newChallenge() would do.
That avoids unnecessary disk writes for storing the new nonce, which
is important for local syncs in IVI. It also avoids debug output about
a challenge that would not get sent.
|
|
Package name and version should be passed to AC_INIT according to
current Automake documentation.
In addition, we need to pass subdir-objects as option to
AM_INIT_AUTOMAKE to avoid warnings about the location of our object
files (first seen with automake 1.14.1).
|
|
|
|
When the client already knows that it is never going to sync again or
the next sync will have to be a slow sync anyway, then the binfile
client files are not needed. Suppressing writing of them is useful on
devices with flash memory to avoid flash wearout. The specific use
case is PBAP caching in a car's head unit.
When setting "/dev/null" as datadir, the platform layer will avoid
creating the files and instead keep their content in memory. This is
required (and sufficient) for syncing once, which will close and later
read from some files at least once. Therefore the data cannot be
thrown away completely.
|
|
clang 3.4 warned about unused global variables.
|
|
Item operations returning LOCERR_AGAIN do not need to be logged
as error. This will trigger logging elsewhere, so don't log it at
all in the binfile layer.
|
|
clang 3.4 points out that the ! in !aQuoteMode!=qm_none in
quoteStringAppend() only gets applied to aQuoteMode. Explicit
brackets silence the compiler warning without changing the
expression.
The compiler indeed points out some unusual code. The if branch is
taken for aQuoteMode == qm_none (probably what was intended) because
!qm_none != qm_none with qm_none == 0 and not taken for all other
values where !aQuoteMode is false and thus equal to qm_none.
It probably would be simpler to write "if (aQuoteMode == qm_none)" but
I don't want to change the meaning of the code.
|
|
String fields also get used for arbitrary binary data, like
photos. In that case we need to compare the entire std::string,
not just the part before any embedded null byte.
This gets done using std::string::compare. Case-insensitive
comparison still uses C strucmp() and thus should never be used
for non-string data.
|
|
gcc 4.9 found a flaw in the code path for updating an item when the DB
detects a duplicate: some variables where not preserved across
function restarts.
|
|
gcc 4.9 warns about initializing a boolean with NULL. Probably
should have been false to start with.
|
|
fIsStringBLOB is only used in pluginapids. Found by gcc 4.9.
|
|
gcc 4.9 warns that "if (ext == 255)" is always false due to casting
rules. Need to convert 255 into the same type as ext, as done
elsewhere.
|
|
gcc 4.9 warns about the unused local variables. sizeof() does not need
real variables, so use a casted NULL pointer instead and get rid of
the variables and the warning.
|
|
gcc 4.9 warns about uninitialized tctx. It doesn't seem to notice that
the code using it is not reached unless ISO8601StrToTimestamp()
succeeds and initializes the variable first.
Suppress the warning by initializing to zero first.
|
|
cppcheck 1.65 warns about uninitialized variables in the "retry function" code
blocks. These are false positives because the code is only reached after
initializing them and/or restoring unitialized variables would not matter.
Have to add inline suppressions to get rid of the warnings.
|
|
cppcheck 1.65 warned about uninitialized variables because it did not fully
understand the indirect "if no file, then error, if no error, then use file"
logic.
Simplifying the code to check the file directly avoids the false positives
and also is easier to read for humans.
|
