Subject: Re: CVS commit: pkgsrc/devel/libgphoto2
To: Stephen Borrill <sborrill@NetBSD.org>
From: Bernd Ernesti <veego@NetBSD.org>
List: pkgsrc-changes
Date: 03/24/2007 00:31:07
--pf9I7BMVVzbSWLtt
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

Hmmm,

I also tracked it down to

https://sourceforge.net/tracker/?func=detail&atid=108874&aid=1648398&group_id=8874

which uses a different patch which was allready commited to the libgphoto2
source code.

It would be better to use rev 9964 (there were some further commits, but they
were backout out).

See attached patch (which fixed two files in one patch). I didn't had the time
to commit this patch.

Bernd

On Mon, Mar 19, 2007 at 08:42:05AM +0000, Stephen Borrill wrote:
> 
> Module Name:	pkgsrc
> Committed By:	sborrill
> Date:		Mon Mar 19 08:42:04 UTC 2007
> 
> Modified Files:
> 	pkgsrc/devel/libgphoto2: Makefile distinfo
> Added Files:
> 	pkgsrc/devel/libgphoto2/patches: patch-aa
> 
> Log Message:
> Patch ptp code to work with (hopefully) all implementations of iconv,
> including NetBSD in-tree, pkgsrc and glibc.
> 
> 
> To generate a diff of this commit:
> cvs rdiff -r1.34 -r1.35 pkgsrc/devel/libgphoto2/Makefile
> cvs rdiff -r1.10 -r1.11 pkgsrc/devel/libgphoto2/distinfo
> cvs rdiff -r0 -r1.5 pkgsrc/devel/libgphoto2/patches/patch-aa
> 
> Please note that diffs are not public domain; they are subject to the
> copyright notices on the relevant files.
> 

--pf9I7BMVVzbSWLtt
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename=patch-ag

https://sourceforge.net/tracker/?func=detail&atid=108874&aid=1648398&group_id=8874
Fixed in rev 9964.

--- camlibs/ptp2/library.c_ORIG	2007-01-30 20:06:58.000000000 -0500
+++ camlibs/ptp2/library.c	2007-01-30 20:13:25.000000000 -0500
@@ -50,23 +50,6 @@
 #  define N_(String) (String)
 #endif
 
-/*
- * On MacOS (Darwin) and *BSD we're not using glibc, but libiconv.
- * glibc knows that UCS-2 is to be in the local machine endianness,
- * whereas libiconv does not. So we construct this macro to get
- * things right. Reportedly, glibc 2.1.3 has a bug so that UCS-2
- * is always bigendian though, we would need to work around that
- * too...
- */
-#ifndef __GLIBC__
-#define UCS_2_INTERNAL "UCS-2-INTERNAL"
-#else
-#if (__GLIBC__ == 2 && __GLIBC_MINOR__ <= 1 )
-#error "Too old glibc. This versions iconv() implementation cannot be trusted."
-#endif
-#define UCS_2_INTERNAL "UCS-2"
-#endif
-
 #include "ptp.h"
 #include "ptp-bugs.h"
 #include "ptp-private.h"
@@ -3619,7 +3602,7 @@
     	CameraAbilities a;
 	int ret, i, retried = 0;
 	PTPParams *params;
-	char *curloc;
+	char *camloc, *curloc;
 
 	/* Make sure our port is either USB or PTP/IP. */
 	if ((camera->port->type != GP_PORT_USB) && (camera->port->type != GP_PORT_PTPIP)) {
@@ -3649,6 +3632,11 @@
 	memset (camera->pl->params.data, 0, sizeof (PTPData));
 	((PTPData *) camera->pl->params.data)->camera = camera;
 	camera->pl->params.byteorder = PTP_DL_LE;
+    if (camera->pl->params.byteorder == PTP_DL_LE) {
+        camloc = "UCS-2LE";
+    } else {
+        camloc = "UCS-2BE";
+    }
 
 	switch (camera->port->type) {
 	case GP_PORT_USB:
@@ -3696,8 +3684,8 @@
 
 	curloc = nl_langinfo (CODESET);
 	if (!curloc) curloc="UTF-8";
-	camera->pl->params.cd_ucs2_to_locale = iconv_open(curloc, UCS_2_INTERNAL);
-	camera->pl->params.cd_locale_to_ucs2 = iconv_open(UCS_2_INTERNAL, curloc);
+	camera->pl->params.cd_ucs2_to_locale = iconv_open(curloc, camloc);
+	camera->pl->params.cd_locale_to_ucs2 = iconv_open(camloc, curloc);
 	if ((camera->pl->params.cd_ucs2_to_locale == (iconv_t) -1) ||
 	    (camera->pl->params.cd_locale_to_ucs2 == (iconv_t) -1)) {
 		gp_log (GP_LOG_ERROR, "iconv", "Failed to create iconv converter.\n");
--- camlibs/ptp2/ptp-pack.c_ORIG	2007-01-30 20:06:47.000000000 -0500
+++ camlibs/ptp2/ptp-pack.c	2007-01-30 21:32:05.000000000 -0500
@@ -89,41 +89,38 @@
 static inline char*
 ptp_unpack_string(PTPParams *params, unsigned char* data, uint16_t offset, uint8_t *len)
 {
-	int i;
-	uint8_t loclen;
+	uint8_t length;
+	uint16_t string[PTP_MAXSTRLEN+1];
+	/* allow for UTF-8: max of 3 bytes per UCS-2 char, plus final null */
+	char loclstr[PTP_MAXSTRLEN*3+1]; 
+	size_t nconv, srclen, destlen;
+	char *src, *dest;
+
+	length = dtoh8a(&data[offset]);	/* PTP_MAXSTRLEN == 255, 8 bit len */
+	*len = length;
+	if (length == 0)		/* nothing to do? */
+		return(NULL);
+
+	/* copy to string[] to ensure correct alignment for iconv(3) */
+	memcpy(string, &data[offset+1], length * sizeof(string[0]));
+	string[length] = 0x0000U;   /* be paranoid!  add a terminator. */
+	loclstr[0] = '\0';
+    
+	/* convert from camera UCS-2 to our locale */
+	src = (char *)string;
+	srclen = length * sizeof(string[0]);
+	dest = loclstr;
+	destlen = sizeof(loclstr)-1;
+	nconv = iconv(params->cd_ucs2_to_locale, &src, &srclen, 
+			&dest, &destlen);
+	if (nconv == (size_t) -1)
+		return(NULL);
 
-	/* Cannot exceed 255 (PTP_MAXSTRLEN) since it is a single byte, duh ... */
-	loclen = dtoh8a(&data[offset]);
-	/* This len is used to advance the buffer pointer */
-	*len = loclen;
-	if (loclen) {
-		uint16_t string[PTP_MAXSTRLEN+1];
-		char *stringp = (char *) string;
-		char loclstr[PTP_MAXSTRLEN*3+1]; /* UTF-8 encoding is max 3 bytes per UCS2 char. */
-		char *locp = loclstr;
-		size_t nconv;
-		size_t convlen = loclen * 2; /* UCS-2 is 16 bit wide */
-		size_t convmax = PTP_MAXSTRLEN*3;
-		
-		for (i=0;i<loclen;i++) {
-			string[i]=dtoh16a(&data[offset+i*2+1]);
-		}
-		/* be paranoid! Add a terminator. :( */
-		string[loclen]=0x0000U;
-		loclstr[0]='\0';
-		/* loclstr=ucs2_to_utf8(string); */
-		/* Do the conversion.  */
-		nconv = iconv (params->cd_ucs2_to_locale, &stringp, &convlen, &locp, &convmax);
-		/* FIXME: handle size errors */
-		loclstr[PTP_MAXSTRLEN*3] = '\0';
-		if (nconv == (size_t) -1)
-			return NULL;
-		return strdup(loclstr);
-	}
-	return NULL;
+	*dest = '\0';
+	loclstr[sizeof(loclstr)-1] = '\0';   /* be safe? */
+	return(strdup(loclstr));
 }
 
-
 static inline int
 ucs2strlen(uint16_t const * const unicstr)
 {
@@ -138,7 +135,6 @@
 static inline void
 ptp_pack_string(PTPParams *params, char *string, unsigned char* data, uint16_t offset, uint8_t *len)
 {
-	int i;
 	int packedlen;
 	uint16_t ucs2str[PTP_MAXSTRLEN+1];
 	char *ucs2strp = (char *) ucs2str;
@@ -148,11 +144,15 @@
 	size_t convmax = PTP_MAXSTRLEN * 2; /* Includes the terminator */
 
 	/* Cannot exceed 255 (PTP_MAXSTRLEN) since it is a single byte, duh ... */
-	ucs2str[0] = 0x0000U;
-	memset(ucs2strp, 0, PTP_MAXSTRLEN*2+2);
-	nconv = iconv (params->cd_locale_to_ucs2, &stringp, &convlen, &ucs2strp, &convmax);
+	memset(ucs2strp, 0, sizeof(ucs2str));  /* XXX: necessary? */
+	nconv = iconv(params->cd_locale_to_ucs2, &stringp, &convlen,
+		&ucs2strp, &convmax);
 	if (nconv == (size_t) -1)
 		ucs2str[0] = 0x0000U;
+	/*
+	 * XXX: isn't packedlen just ( (uint16_t *)ucs2strp - ucs2str )?
+	 *      why do we need ucs2strlen()?
+	 */
 	packedlen = ucs2strlen(ucs2str);
 	if (packedlen > PTP_MAXSTRLEN-1) {
 		*len=0;
@@ -161,10 +161,8 @@
 	
 	/* number of characters including terminating 0 (PTP standard confirmed) */
 	htod8a(&data[offset],packedlen+1);
-	for (i=0;i<packedlen && i< PTP_MAXSTRLEN; i++) {
-		htod16a(&data[offset+i*2+1],ucs2str[i]);
-	}
-	htod16a(&data[offset+i*2+1],0x0000);
+	memcpy(&data[offset+1], &ucs2str[0], packedlen * sizeof(ucs2str[0]));
+	htod16a(&data[offset+packedlen*2+1], 0x0000);  /* terminate 0 */
 
 	/* The returned length is in number of characters */
 	*len = (uint8_t) packedlen+1;

--pf9I7BMVVzbSWLtt--