Bug#339344: Patch for wrong pointer type
Wouter Verhelst [EMAIL PROTECTED] writes: tags 339344 - patch thanks On Mon, Jul 17, 2006 at 09:56:10AM +0200, Goswin Brederlow wrote: Package: belpic Followup-For: Bug #339344 Hi, attached is a patch for the pointer conversion error. It doesn't actually work, though; you can't change the definition of a class which is rather central in the entire class hierarchy and hope that things still compile. Also, this implies an API change, which is a Very Bad Thing(TM). Worked fine here on amd64. What error do you get? The right fix is simpler: rather than trying to cast unsigned long to unsigned int, one should cast unsigned long to size_t (which happens to be the same thing on 32bit architectures, but nowhere else). That isn't right. That might just be working on the relevant systems. MfG Goswin -- To UNSUBSCRIBE, email to [EMAIL PROTECTED] with a subject of unsubscribe. Trouble? Contact [EMAIL PROTECTED]
Bug#339344: Patch for wrong pointer type
On Tue, Jul 25, 2006 at 01:21:44PM +0200, Goswin von Brederlow wrote: Wouter Verhelst [EMAIL PROTECTED] writes: tags 339344 - patch thanks On Mon, Jul 17, 2006 at 09:56:10AM +0200, Goswin Brederlow wrote: Package: belpic Followup-For: Bug #339344 Hi, attached is a patch for the pointer conversion error. It doesn't actually work, though; you can't change the definition of a class which is rather central in the entire class hierarchy and hope that things still compile. Also, this implies an API change, which is a Very Bad Thing(TM). Worked fine here on amd64. What error do you get? The exact reverse of what you got on amd64 without the fix. Unfixed, the code assumed that size_t == unsigned int, and that one cast broke on architectures where this is not true. Your fix basically assumed that size_t == unsigned long (the code refers to that modified data later on as size_t anyway). It also happened to change the ABI (not just the API) of the library, which would have required me to do a SONAME bump (the function was part of an abstract class, and many child classes reimplement it). The right fix is simpler: rather than trying to cast unsigned long to unsigned int, one should cast unsigned long to size_t (which happens to be the same thing on 32bit architectures, but nowhere else). That isn't right. That might just be working on the relevant systems. Sorry, after looking at the code for over three hours, I came to the conclusion that it is. Please check the patch and prove to me that it is wrong, or it will stay the way it is now. -- Fun will now commence -- Seven Of Nine, Ashes to Ashes, stardate 53679.4 -- To UNSUBSCRIBE, email to [EMAIL PROTECTED] with a subject of unsubscribe. Trouble? Contact [EMAIL PROTECTED]
Bug#339344: Patch for wrong pointer type
Wouter Verhelst [EMAIL PROTECTED] writes: On Tue, Jul 25, 2006 at 01:21:44PM +0200, Goswin von Brederlow wrote: Wouter Verhelst [EMAIL PROTECTED] writes: tags 339344 - patch thanks On Mon, Jul 17, 2006 at 09:56:10AM +0200, Goswin Brederlow wrote: Package: belpic Followup-For: Bug #339344 Hi, attached is a patch for the pointer conversion error. It doesn't actually work, though; you can't change the definition of a class which is rather central in the entire class hierarchy and hope that things still compile. Also, this implies an API change, which is a Very Bad Thing(TM). Worked fine here on amd64. What error do you get? The exact reverse of what you got on amd64 without the fix. Unfixed, the code assumed that size_t == unsigned int, and that one cast broke on architectures where this is not true. Your fix basically assumed that size_t == unsigned long (the code refers to that modified data later on as size_t anyway). It also My fix assumes size_t is size_t. As you said the data is used as size_t later on and I just made it declare the argument as size_t instead of unsigned long. You probably run into problems on the calling side of the function where someone used an unsigned long pointer instead of a size_t pointer. That is why I asked for the error. happened to change the ABI (not just the API) of the library, which would have required me to do a SONAME bump (the function was part of an abstract class, and many child classes reimplement it). The API/ABI change was always a given for me. The right fix is simpler: rather than trying to cast unsigned long to unsigned int, one should cast unsigned long to size_t (which happens to be the same thing on 32bit architectures, but nowhere else). That isn't right. That might just be working on the relevant systems. Sorry, after looking at the code for over three hours, I came to the conclusion that it is. Please check the patch and prove to me that it is wrong, or it will stay the way it is now. Consider a system where size_t != unsigned long. I can't name you an example in Debian but it isn't impossible. Point is that you are casting the type of a pointer and they may not match. I'm not sure but I think Mips N32 abi has this case. 32 bit pointers but 64bit longs, size_t should be 32bit. The software would fail there. MfG Goswin -- To UNSUBSCRIBE, email to [EMAIL PROTECTED] with a subject of unsubscribe. Trouble? Contact [EMAIL PROTECTED]
Bug#339344: Patch for wrong pointer type
tags 339344 - patch thanks On Mon, Jul 17, 2006 at 09:56:10AM +0200, Goswin Brederlow wrote: Package: belpic Followup-For: Bug #339344 Hi, attached is a patch for the pointer conversion error. It doesn't actually work, though; you can't change the definition of a class which is rather central in the entire class hierarchy and hope that things still compile. Also, this implies an API change, which is a Very Bad Thing(TM). The right fix is simpler: rather than trying to cast unsigned long to unsigned int, one should cast unsigned long to size_t (which happens to be the same thing on 32bit architectures, but nowhere else). -- Fun will now commence -- Seven Of Nine, Ashes to Ashes, stardate 53679.4 -- To UNSUBSCRIBE, email to [EMAIL PROTECTED] with a subject of unsubscribe. Trouble? Contact [EMAIL PROTECTED]
Bug#339344: Patch for wrong pointer type
Package: belpic
Followup-For: Bug #339344
Hi,
attached is a patch for the pointer conversion error.
MfG
Goswin
-- System Information:
Debian Release: testing/unstable
APT prefers unstable
APT policy: (500, 'unstable')
Architecture: amd64 (x86_64)
Shell: /bin/sh linked to /bin/bash
Kernel: Linux 2.6.8-frosties-2
Locale: LANG=C, LC_CTYPE=C (charmap=ANSI_X3.4-1968)
diff -u belpic-2.5.9/debian/changelog belpic-2.5.9/debian/changelog
--- belpic-2.5.9/debian/changelog
+++ belpic-2.5.9/debian/changelog
@@ -1,3 +1,9 @@
+belpic (2.5.9-1a0.mrvn.1) unstable; urgency=low
+
+ * size_t fix
+
+ -- Goswin von Brederlow [EMAIL PROTECTED] Mon, 17 Jul 2006 09:50:00 +0200
+
belpic (2.5.9-1) unstable; urgency=low
* New upstream release.
only in patch2:
unchanged:
--- belpic-2.5.9.orig/src/eidlib/OpenSCReader.h
+++ belpic-2.5.9/src/eidlib/OpenSCReader.h
@@ -34,7 +34,7 @@
virtual long Transmit(const unsigned char *pucSend, int iSendLen, unsigned char *pucRecv, unsigned long *pulRecvLen, BEID_Status *ptStatus);
virtual long BeginTransaction(BEID_Status *ptStatus);
virtual long EndTransaction(BEID_Status *ptStatus);
-virtual long ReadFile(const unsigned char *ucFile, int iFileLen, unsigned char *pucOutput, unsigned long *pulOutLen, BEID_Status *ptStatus);
+virtual long ReadFile(const unsigned char *ucFile, int iFileLen, unsigned char *pucOutput, size_t *pulOutLen, BEID_Status *ptStatus);
virtual long SelectFile(const unsigned char *ucFile, int iFileLen, unsigned char ucP1, BEID_Status *ptStatus);
virtual long ReadBinary(unsigned char *pszOutput, unsigned long *pulOutLen, BEID_Status *ptStatus, int iOffset = 0, int iCount = MAX_FILE_SIZE);
virtual long UpdateBinary(unsigned char *pucInput, unsigned long ulInLen, BEID_Status *ptStatus);
only in patch2:
unchanged:
--- belpic-2.5.9.orig/src/eidlib/OpenSCReader.cpp
+++ belpic-2.5.9/src/eidlib/OpenSCReader.cpp
@@ -174,7 +174,7 @@
return lRet;
}
-long COpenSCReader::ReadFile(const unsigned char *ucFile, int iFileLen, unsigned char *pucOutput, unsigned long *pulOutLen, BEID_Status *ptStatus)
+long COpenSCReader::ReadFile(const unsigned char *ucFile, int iFileLen, unsigned char *pucOutput, size_t *pulOutLen, BEID_Status *ptStatus)
{
long lRet = SC_NO_ERROR;
if(m_p15Card != NULL)
@@ -183,7 +183,7 @@
sc_append_path_id(tPath, ucFile, iFileLen);
tPath.count = -1;
tPath.type = SC_PATH_TYPE_PATH;
-if(SC_NO_ERROR != (lRet = sc_pkcs15_read_file2(m_p15Card, tPath, pucOutput, (unsigned int*)pulOutLen, NULL)))
+if(SC_NO_ERROR != (lRet = sc_pkcs15_read_file2(m_p15Card, tPath, pucOutput, pulOutLen, NULL)))
{
*pulOutLen = 0;
}
