GitLab

"byte" is a pre-defined type recent C++. Better avoid it entirely
and use the custom uInt8 instead.
Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>

A recent g++ on Fedora Rawhide pointed out that the result
of empty() (a boolean) was ignored. Perhaps the code made
sense with some older libc++, but to clear the strings we
would have to use clear().

In case that this is not available in some older libc++, let's use a
string assignment.
Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>

The newer gcc warns about several sprintf usages (suppressed with compiler
flags because the code is hopefully correct) and one buffer size (increased
because that was easy).

Clang correctly pointed out that memset() for a non-trivial type like
sysync::tChangeStruct isn't guaranteed to work. Better overwrite with
an empty instance.
Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>

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" (8a7356c1,
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).

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).