[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-core-devel
Subject:    KPilot critical bugfixes
From:       Adriaan de Groot <groot () kde ! org>
Date:       2005-05-27 14:53:16
Message-ID: 200505271653.16609.groot () kde ! org
[Download RAW message or body]

As usual, we have discovered some pretty critical bugs in KPilot moments - 
days - after the tarballs for a release are created. The memofile conduit 
(@since KDE 3.4.0) will eat all previously synced memos; in addition, 
category names will be broken because the last character of each category 
name is removed (blame qstrncpy() for being more like strlcpy() and less like 
strncpy()).

The attached patch applies from the tag; it fixes both problems. For 
packagers, I'd suggest not packaging KPilot without these fixes.

I swear, I am never touching BRANCH again. It just doesn't work.

-- 
As of September 1st, 2004, the University of Nijmegen will _still_ be the 
University of Nijmegen, but with a different nonsensical adjective in front.
    Reach me at groot@kde.org instead. GPG FEA2 A3FE

["kpilot-tag-to-branch.diff" (text/x-diff)]

Index: conduits/memofileconduit/memofile-conduit.cc
===================================================================
--- conduits/memofileconduit/memofile-conduit.cc	(.../tags/KDE/3.4.1/kdepim/kpilot)	(revision 418726)
+++ conduits/memofileconduit/memofile-conduit.cc	(.../branches/KDE/3.4/kdepim/kpilot)	(revision 418726)
@@ -114,7 +114,7 @@
 	setFirstSync( _memofiles->isFirstSync() );
 	addSyncLogEntry(i18n(" Syncing with %1.").arg(_memo_directory));
 
-	if ( (syncMode() == SyncAction::SyncMode::eCopyHHToPC) || isFirstSync() ) {
+	if ( (syncMode() == SyncAction::SyncMode::eCopyHHToPC) || _memofiles->isFirstSync() ) {
 		addSyncLogEntry(i18n(" Copying Pilot to PC..."));
 #ifdef DEBUG
 		DEBUGCONDUIT << fname << ": copying Pilot to PC." << endl;
@@ -474,17 +474,11 @@
 	// Note: This will reset both fCategories and fMemoAppInfo, so
 	//       after this, we need to reinitialize our memofiles object...
 	setAppInfo();
-	cleanup();
 
 	// re-create our memofiles helper...
 	delete _memofiles;
 	_memofiles = new Memofiles(fCategories, *fMemoAppInfo, _memo_directory);
 
-	// make sure we are starting with a clean database on both ends...
-	fDatabase->deleteRecord(0, true);
-	fLocalDatabase->deleteRecord(0, true);
-	cleanup();
-
 	_memofiles->load(true);
 
 	QPtrList<Memofile> memofiles = _memofiles->getAll();
@@ -496,11 +490,39 @@
 	}
 
 	_memofiles->save();
-
+	
+	// now that we've copied from the PC to our handheld, remove anything extra from the
+	// handheld...
+	deleteUnsyncedHHRecords();
+	
 	return true;
 
 }
 
+void MemofileConduit::deleteUnsyncedHHRecords()
+{
+	FUNCTIONSETUP;
+	if ( syncMode()==SyncMode::eCopyPCToHH )
+	{
+		RecordIDList ids=fDatabase->idList();
+		RecordIDList::iterator it;
+		for ( it = ids.begin(); it != ids.end(); ++it )
+		{
+			if (!_memofiles->find(*it))
+			{
+#ifdef DEBUG
+				DEBUGCONDUIT << fname
+				<< "Deleting record with ID "<<*it <<" from handheld "
+				<< "(is not on PC, and syncing with PC->HH direction)"
+				<< endl;
+#endif
+				fDatabase->deleteRecord(*it);
+				fLocalDatabase->deleteRecord(*it);
+			}
+		}
+	}
+}
+
 int MemofileConduit::writeToPilot(Memofile * memofile)
 {
 	FUNCTIONSETUP;
Index: conduits/memofileconduit/memofile-conduit.h
===================================================================
--- conduits/memofileconduit/memofile-conduit.h	(.../tags/KDE/3.4.1/kdepim/kpilot)	(revision 418726)
+++ conduits/memofileconduit/memofile-conduit.h	(.../branches/KDE/3.4/kdepim/kpilot)	(revision 418726)
@@ -88,6 +88,7 @@
 
 	bool	copyHHToPC();
 	bool	copyPCToHH();
+	void	deleteUnsyncedHHRecords();
 	bool	sync();
 
 	int 	writeToPilot(Memofile * memofile);
Index: lib/pilotDatabase.cc
===================================================================
--- lib/pilotDatabase.cc	(.../tags/KDE/3.4.1/kdepim/kpilot)	(revision 418726)
+++ lib/pilotDatabase.cc	(.../branches/KDE/3.4/kdepim/kpilot)	(revision 418726)
@@ -185,7 +185,7 @@
 	int len = CATEGORY_NAME_SIZE - 1;
 	QCString t = PilotAppCategory::codec()->fromUnicode(s,len);
 	memset(categoryInfo()->name[i],0,CATEGORY_NAME_SIZE);
-	qstrncpy(categoryInfo()->name[i],t,kMin(len,(int)CATEGORY_NAME_SIZE));
+	qstrncpy(categoryInfo()->name[i],t,CATEGORY_NAME_SIZE);
 	return true;
 }
 
Index: lib/data/MemoDB.pdb
===================================================================
Cannot display: file marked as a binary type.
svn:mime-type = application/octet-stream

Property changes on: lib/data/MemoDB.pdb
___________________________________________________________________
Name: svn:mime-type
   + application/octet-stream

Index: lib/pilotLocalDatabase.cc
===================================================================
--- lib/pilotLocalDatabase.cc	(.../tags/KDE/3.4.1/kdepim/kpilot)	(revision 418726)
+++ lib/pilotLocalDatabase.cc	(.../branches/KDE/3.4/kdepim/kpilot)	(revision 418726)
@@ -618,7 +618,7 @@
 
 	int count;
 	pi_file_get_entries(dbFile, &count);
-	if (count > 0)
+	if (count >= 0)
 	{
 		KPILOT_DELETE(d);
 		d = new Private(count);
Index: lib/pilotDatabase.h
===================================================================
--- lib/pilotDatabase.h	(.../tags/KDE/3.4.1/kdepim/kpilot)	(revision 418726)
+++ lib/pilotDatabase.h	(.../branches/KDE/3.4/kdepim/kpilot)	(revision 418726)
@@ -279,9 +279,12 @@
 		int appLen = MAX_APPINFO_SIZE;
 		unsigned char buffer[MAX_APPINFO_SIZE];
 
-		appLen = d->readAppBlock(buffer,appLen);
-
-		(*unpack)(&fInfo, buffer, appLen);
+		if (d && d->isDBOpen())
+		{
+			appLen = d->readAppBlock(buffer,appLen);
+			(*unpack)(&fInfo, buffer, appLen);
+		}
+		// fInfo is just a struct, so we can point to it anyway.
 		init(&fInfo.category,appLen);
 	} ;
 
@@ -289,6 +292,10 @@
 	{
 		FUNCTIONSETUP;
 		unsigned char buffer[MAX_APPINFO_SIZE];
+		if (!d || !d->isDBOpen())
+		{
+			return -1;
+		}
 		int appLen = (*pack)(&fInfo, buffer, length());
 		if (appLen > 0)
 		{


[prev in list] [next in list] [prev in thread] [next in thread]