[SCM] Samba Shared Repository - branch v3-6-test updated

Jeremy Allison jra at samba.org
Wed Mar 16 11:44:27 MDT 2011


The branch, v3-6-test has been updated
       via  f88484a s3-printing: fix memory leak in print_cups.c
       via  2d05e26 s3-printing: remove duplicate cups response processing code
       via  e7b59b0 s3-printing: use printcap IDL for IPC
       via  ac311ed idl: define printcap IPC message format
      from  11a258d s3: Use jenkins hash for str_checksum, fix bug 8010

http://gitweb.samba.org/?p=samba.git;a=shortlog;h=v3-6-test


- Log -----------------------------------------------------------------
commit f88484a347eec75e1f05f1538cdd8316d95da714
Author: David Disseldorp <ddiss at suse.de>
Date:   Wed Mar 9 15:18:22 2011 +0100

    s3-printing: fix memory leak in print_cups.c
    
    As found by valgrind, tmp_pcap_cache is not freed following printer list
    tdb update.
    
    Signed-off-by: Andreas Schneider <asn at samba.org>
    
    Autobuild-User: Andreas Schneider <asn at cryptomilk.org>
    Autobuild-Date: Wed Mar 16 16:37:58 CET 2011 on sn-devel-104
    (cherry picked from commit 97cdf15f0905039ca76a40093c712db8b0984caa)

commit 2d05e2675517a8aa9fb9051b0f2871d7e1c8d5c1
Author: David Disseldorp <ddiss at suse.de>
Date:   Wed Mar 9 14:05:39 2011 +0100

    s3-printing: remove duplicate cups response processing code
    
    There is currently a lot of duplicate code included for processing
    responses to CUPS_GET_PRINTERS and CUPS_GET_CLASSES requests. This
    change splits this code into a separate function.
    
    Signed-off-by: Andreas Schneider <asn at samba.org>
    (cherry picked from commit 52845c1054941e697143940b94a0792f4d4e07c5)

commit e7b59b0b563db801edf12083003f95576b7b6df3
Author: David Disseldorp <ddiss at suse.de>
Date:   Tue Mar 8 16:36:03 2011 +0100

    s3-printing: use printcap IDL for IPC
    
    Use printcap IDL for marshalling and unmarshalling messages between cups
    child and parent smbd processes. This simplifies the IPC and ensures
    the parent is notified of cups errors encountered by the child.
    
    https://bugzilla.samba.org/show_bug.cgi?id=7994
    Signed-off-by: Andreas Schneider <asn at samba.org>
    (cherry picked from commit d6cb4feae1eab22d63b42eb5c480578fb1ee99bf)

commit ac311ed7b615ebede4c3e1d44768ae11880665fc
Author: David Disseldorp <ddiss at suse.de>
Date:   Mon Mar 7 15:32:02 2011 +0100

    idl: define printcap IPC message format
    
    Signed-off-by: Andreas Schneider <asn at samba.org>
    (cherry picked from commit 9ea602741934f4e546147fa238332644e8e9f316)

-----------------------------------------------------------------------

Summary of changes:
 librpc/idl/printcap.idl       |   17 ++
 librpc/idl/wscript_build      |    3 +-
 librpc/wscript_build          |    5 +
 source3/Makefile.in           |    2 +-
 source3/printing/print_cups.c |  442 +++++++++++++++++------------------------
 source3/wscript_build         |    1 +
 6 files changed, 205 insertions(+), 265 deletions(-)
 create mode 100644 librpc/idl/printcap.idl


Changeset truncated at 500 lines:

diff --git a/librpc/idl/printcap.idl b/librpc/idl/printcap.idl
new file mode 100644
index 0000000..5ab380c
--- /dev/null
+++ b/librpc/idl/printcap.idl
@@ -0,0 +1,17 @@
+#include "idl_types.h"
+[
+	pointer_default(unique)
+]
+interface printcap
+{
+	typedef struct {
+		[charset(UTF8),string] uint8 *name;
+		[charset(UTF8),string] uint8 *info;
+	} pcap_printer;
+
+	typedef [public] struct {
+		NTSTATUS status;
+		uint32 count;
+		[size_is(count)] pcap_printer printers[];
+	} pcap_data;
+}
diff --git a/librpc/idl/wscript_build b/librpc/idl/wscript_build
index 08fe65f..1a5bc36 100644
--- a/librpc/idl/wscript_build
+++ b/librpc/idl/wscript_build
@@ -10,7 +10,8 @@ bld.SAMBA_PIDL_LIST('PIDL',
                        dbgidl.idl dnsserver.idl echo.idl frsrpc.idl lsa.idl nbt.idl dns.idl
                        oxidresolver.idl samr.idl srvsvc.idl winreg.idl dcerpc.idl
                        drsblobs.idl efs.idl frstrans.idl mgmt.idl netlogon.idl
-                       policyagent.idl scerpc.idl svcctl.idl wkssvc.idl eventlog6.idl backupkey.idl''',
+                       policyagent.idl scerpc.idl svcctl.idl wkssvc.idl eventlog6.idl backupkey.idl
+                       printcap.idl''',
                     options='--header --ndr-parser --samba3-ndr-server --server --client --python',
                     output_dir='../gen_ndr')
 
diff --git a/librpc/wscript_build b/librpc/wscript_build
index 287e587..f75eb87 100644
--- a/librpc/wscript_build
+++ b/librpc/wscript_build
@@ -88,6 +88,11 @@ bld.SAMBA_SUBSYSTEM('NDR_SPOOLSS_BUF',
         deps='talloc'
 	)
 
+bld.SAMBA_SUBSYSTEM('NDR_PRINTCAP',
+	source='gen_ndr/ndr_printcap.c',
+	public_deps='ndr'
+	)
+
 bld.SAMBA_SUBSYSTEM('NDR_EPMAPPER',
 	source='gen_ndr/ndr_epmapper.c',
 	public_deps='ndr'
diff --git a/source3/Makefile.in b/source3/Makefile.in
index f52e922..c1f5205 100644
--- a/source3/Makefile.in
+++ b/source3/Makefile.in
@@ -935,7 +935,7 @@ PRINTING_OBJ = printing/pcap.o printing/print_svid.o printing/print_aix.o \
                printing/print_cups.o printing/print_generic.o \
                printing/lpq_parse.o printing/load.o \
                printing/print_iprint.o printing/print_standard.o \
-               printing/printer_list.o
+               printing/printer_list.o librpc/gen_ndr/ndr_printcap.o
 
 PRINTBASE_OBJ = printing/notify.o printing/printing_db.o
 PRINTBACKEND_OBJ = printing/printing.o \
diff --git a/source3/printing/print_cups.c b/source3/printing/print_cups.c
index 3031a88..e3b08b7 100644
--- a/source3/printing/print_cups.c
+++ b/source3/printing/print_cups.c
@@ -25,6 +25,7 @@
 #include "includes.h"
 #include "printing.h"
 #include "printing/pcap.h"
+#include "librpc/gen_ndr/ndr_printcap.h"
 
 #ifdef HAVE_CUPS
 #include <cups/cups.h>
@@ -112,60 +113,153 @@ static http_t *cups_connect(TALLOC_CTX *frame)
 	return http;
 }
 
-static void send_pcap_info(const char *name, const char *info, void *pd)
+static bool send_pcap_blob(DATA_BLOB *pcap_blob, int fd)
 {
-	int fd = *(int *)pd;
-	size_t namelen = name ? strlen(name)+1 : 0;
-	size_t infolen = info ? strlen(info)+1 : 0;
-
-	DEBUG(11,("send_pcap_info: writing namelen %u\n", (unsigned int)namelen));
-	if (sys_write(fd, &namelen, sizeof(namelen)) != sizeof(namelen)) {
-		DEBUG(10,("send_pcap_info: namelen write failed %s\n",
-			strerror(errno)));
-		return;
-	}
-	DEBUG(11,("send_pcap_info: writing infolen %u\n", (unsigned int)infolen));
-	if (sys_write(fd, &infolen, sizeof(infolen)) != sizeof(infolen)) {
-		DEBUG(10,("send_pcap_info: infolen write failed %s\n",
-			strerror(errno)));
-		return;
-	}
-	if (namelen) {
-		DEBUG(11,("send_pcap_info: writing name %s\n", name));
-		if (sys_write(fd, name, namelen) != namelen) {
-			DEBUG(10,("send_pcap_info: name write failed %s\n",
-				strerror(errno)));
-			return;
-		}
+	size_t ret;
+
+	ret = sys_write(fd, &pcap_blob->length, sizeof(pcap_blob->length));
+	if (ret != sizeof(pcap_blob->length)) {
+		return false;
+	}
+
+	ret = sys_write(fd, pcap_blob->data, pcap_blob->length);
+	if (ret != pcap_blob->length) {
+		return false;
+	}
+
+	DEBUG(10, ("successfully sent blob of len %ld\n", (int64_t)ret));
+	return true;
+}
+
+static bool recv_pcap_blob(TALLOC_CTX *mem_ctx, int fd, DATA_BLOB *pcap_blob)
+{
+	size_t blob_len;
+	size_t ret;
+
+	ret = sys_read(fd, &blob_len, sizeof(blob_len));
+	if (ret != sizeof(blob_len)) {
+		return false;
+	}
+
+	*pcap_blob = data_blob_talloc_named(mem_ctx, NULL, blob_len,
+					   "cups pcap");
+	if (pcap_blob->length != blob_len) {
+		return false;
+	}
+	ret = sys_read(fd, pcap_blob->data, blob_len);
+	if (ret != blob_len) {
+		talloc_free(pcap_blob->data);
+		return false;
 	}
-	if (infolen) {
-		DEBUG(11,("send_pcap_info: writing info %s\n", info));
-		if (sys_write(fd, info, infolen) != infolen) {
-			DEBUG(10,("send_pcap_info: info write failed %s\n",
-				strerror(errno)));
-			return;
+
+	DEBUG(10, ("successfully recvd blob of len %ld\n", (int64_t)ret));
+	return true;
+}
+
+static bool process_cups_printers_response(TALLOC_CTX *mem_ctx,
+					   ipp_t *response,
+					   struct pcap_data *pcap_data)
+{
+	ipp_attribute_t	*attr;
+	char *name;
+	char *info;
+	struct pcap_printer *printer;
+	bool ret_ok = false;
+
+	for (attr = response->attrs; attr != NULL;) {
+	       /*
+		* Skip leading attributes until we hit a printer...
+		*/
+
+		while (attr != NULL && attr->group_tag != IPP_TAG_PRINTER)
+			attr = attr->next;
+
+		if (attr == NULL)
+			break;
+
+	       /*
+		* Pull the needed attributes from this printer...
+		*/
+
+		name       = NULL;
+		info       = NULL;
+
+		while (attr != NULL && attr->group_tag == IPP_TAG_PRINTER) {
+			size_t size;
+			if (strcmp(attr->name, "printer-name") == 0 &&
+			    attr->value_tag == IPP_TAG_NAME) {
+				if (!pull_utf8_talloc(mem_ctx,
+						&name,
+						attr->values[0].string.text,
+						&size)) {
+					goto err_out;
+				}
+			}
+
+			if (strcmp(attr->name, "printer-info") == 0 &&
+			    attr->value_tag == IPP_TAG_TEXT) {
+				if (!pull_utf8_talloc(mem_ctx,
+						&info,
+						attr->values[0].string.text,
+						&size)) {
+					goto err_out;
+				}
+			}
+
+			attr = attr->next;
 		}
+
+	       /*
+		* See if we have everything needed...
+		*/
+
+		if (name == NULL)
+			break;
+
+		if (pcap_data->count == 0) {
+			printer = talloc_array(mem_ctx, struct pcap_printer, 1);
+		} else {
+			printer = talloc_realloc(mem_ctx, pcap_data->printers,
+						 struct pcap_printer,
+						 pcap_data->count + 1);
+		}
+		if (printer == NULL) {
+			goto err_out;
+		}
+		pcap_data->printers = printer;
+		pcap_data->printers[pcap_data->count].name = name;
+		pcap_data->printers[pcap_data->count].info = info;
+		pcap_data->count++;
 	}
+
+	ret_ok = true;
+err_out:
+	return ret_ok;
 }
 
+/*
+ * request printer list from cups, send result back to up parent via fd.
+ * returns true if the (possibly failed) result was successfuly sent to parent.
+ */
 static bool cups_cache_reload_async(int fd)
 {
 	TALLOC_CTX *frame = talloc_stackframe();
-	struct pcap_cache *tmp_pcap_cache = NULL;
+	struct pcap_data pcap_data;
 	http_t		*http = NULL;		/* HTTP connection to server */
 	ipp_t		*request = NULL,	/* IPP Request */
 			*response = NULL;	/* IPP Response */
-	ipp_attribute_t	*attr;		/* Current attribute */
 	cups_lang_t	*language = NULL;	/* Default language */
-	char		*name,		/* printer-name attribute */
-			*info;		/* printer-info attribute */
 	static const char *requested[] =/* Requested attributes */
 			{
 			  "printer-name",
 			  "printer-info"
 			};
 	bool ret = False;
-	size_t size;
+	enum ndr_err_code ndr_ret;
+	DATA_BLOB pcap_blob;
+
+	ZERO_STRUCT(pcap_data);
+	pcap_data.status = NT_STATUS_UNSUCCESSFUL;
 
 	DEBUG(5, ("reloading cups printcap cache\n"));
 
@@ -175,10 +269,6 @@ static bool cups_cache_reload_async(int fd)
 
         cupsSetPasswordCB(cups_passwd_cb);
 
-       /*
-	* Try to connect to the server...
-	*/
-
 	if ((http = cups_connect(frame)) == NULL) {
 		goto out;
 	}
@@ -210,68 +300,16 @@ static bool cups_cache_reload_async(int fd)
 		      (sizeof(requested) / sizeof(requested[0])),
 		      NULL, requested);
 
-       /*
-	* Do the request and get back a response...
-	*/
-
 	if ((response = cupsDoRequest(http, request, "/")) == NULL) {
 		DEBUG(0,("Unable to get printer list - %s\n",
 			 ippErrorString(cupsLastError())));
 		goto out;
 	}
 
-	for (attr = response->attrs; attr != NULL;) {
-	       /*
-		* Skip leading attributes until we hit a printer...
-		*/
-
-		while (attr != NULL && attr->group_tag != IPP_TAG_PRINTER)
-			attr = attr->next;
-
-		if (attr == NULL)
-        		break;
-
-	       /*
-		* Pull the needed attributes from this printer...
-		*/
-
-		name       = NULL;
-		info       = NULL;
-
-		while (attr != NULL && attr->group_tag == IPP_TAG_PRINTER) {
-        		if (strcmp(attr->name, "printer-name") == 0 &&
-			    attr->value_tag == IPP_TAG_NAME) {
-				if (!pull_utf8_talloc(frame,
-						&name,
-						attr->values[0].string.text,
-						&size)) {
-					goto out;
-				}
-			}
-
-        		if (strcmp(attr->name, "printer-info") == 0 &&
-			    attr->value_tag == IPP_TAG_TEXT) {
-				if (!pull_utf8_talloc(frame,
-						&info,
-						attr->values[0].string.text,
-						&size)) {
-					goto out;
-				}
-			}
-
-        		attr = attr->next;
-		}
-
-	       /*
-		* See if we have everything needed...
-		*/
-
-		if (name == NULL)
-			break;
-
-		if (!pcap_cache_add_specific(&tmp_pcap_cache, name, info)) {
-			goto out;
-		}
+	ret = process_cups_printers_response(frame, response, &pcap_data);
+	if (!ret) {
+		DEBUG(0,("failed to process cups response\n"));
+		goto out;
 	}
 
 	ippDelete(response);
@@ -302,72 +340,19 @@ static bool cups_cache_reload_async(int fd)
 		      (sizeof(requested) / sizeof(requested[0])),
 		      NULL, requested);
 
-       /*
-	* Do the request and get back a response...
-	*/
-
 	if ((response = cupsDoRequest(http, request, "/")) == NULL) {
 		DEBUG(0,("Unable to get printer list - %s\n",
 			 ippErrorString(cupsLastError())));
 		goto out;
 	}
 
-	for (attr = response->attrs; attr != NULL;) {
-	       /*
-		* Skip leading attributes until we hit a printer...
-		*/
-
-		while (attr != NULL && attr->group_tag != IPP_TAG_PRINTER)
-			attr = attr->next;
-
-		if (attr == NULL)
-        		break;
-
-	       /*
-		* Pull the needed attributes from this printer...
-		*/
-
-		name       = NULL;
-		info       = NULL;
-
-		while (attr != NULL && attr->group_tag == IPP_TAG_PRINTER) {
-        		if (strcmp(attr->name, "printer-name") == 0 &&
-			    attr->value_tag == IPP_TAG_NAME) {
-				if (!pull_utf8_talloc(frame,
-						&name,
-						attr->values[0].string.text,
-						&size)) {
-					goto out;
-				}
-			}
-
-        		if (strcmp(attr->name, "printer-info") == 0 &&
-			    attr->value_tag == IPP_TAG_TEXT) {
-				if (!pull_utf8_talloc(frame,
-						&info,
-						attr->values[0].string.text,
-						&size)) {
-					goto out;
-				}
-			}
-
-        		attr = attr->next;
-		}
-
-	       /*
-		* See if we have everything needed...
-		*/
-
-		if (name == NULL)
-			break;
-
-		if (!pcap_cache_add_specific(&tmp_pcap_cache, name, info)) {
-			goto out;
-		}
+	ret = process_cups_printers_response(frame, response, &pcap_data);
+	if (!ret) {
+		DEBUG(0,("failed to process cups response\n"));
+		goto out;
 	}
 
-	ret = True;
-
+	pcap_data.status = NT_STATUS_OK;
  out:
 	if (response)
 		ippDelete(response);
@@ -378,14 +363,13 @@ static bool cups_cache_reload_async(int fd)
 	if (http)
 		httpClose(http);
 
-	/* Send all the entries up the pipe. */
-	if (tmp_pcap_cache) {
-		pcap_printer_fn_specific(tmp_pcap_cache,
-				send_pcap_info,
-				(void *)&fd);
-
-		pcap_cache_destroy_specific(&tmp_pcap_cache);
+	ret = false;
+	ndr_ret = ndr_push_struct_blob(&pcap_blob, frame, &pcap_data,
+				       (ndr_push_flags_fn_t)ndr_push_pcap_data);
+	if (ndr_ret == NDR_ERR_SUCCESS) {
+		ret = send_pcap_blob(&pcap_blob, fd);
 	}
+
 	TALLOC_FREE(frame);
 	return ret;
 }
@@ -463,125 +447,57 @@ static void cups_async_callback(struct event_context *event_ctx,
 {
 	TALLOC_CTX *frame = talloc_stackframe();
 	struct cups_async_cb_args *cb_args = (struct cups_async_cb_args *)p;
-	int fd = cb_args->pipe_fd;
 	struct pcap_cache *tmp_pcap_cache = NULL;
-	bool ret_ok = true;
+	bool ret_ok;
+	struct pcap_data pcap_data;
+	DATA_BLOB pcap_blob;
+	enum ndr_err_code ndr_ret;
+	int i;
 
 	DEBUG(5,("cups_async_callback: callback received for printer data. "
-		"fd = %d\n", fd));
+		"fd = %d\n", cb_args->pipe_fd));
 
-	while (1) {
-		char *name = NULL, *info = NULL;
-		size_t namelen = 0, infolen = 0;
-		ssize_t ret = -1;
-
-		ret = sys_read(fd, &namelen, sizeof(namelen));
-		if (ret == 0) {
-			/* EOF */
-			break;
-		}
-		if (ret != sizeof(namelen)) {
-			DEBUG(10,("cups_async_callback: namelen read failed %d %s\n",
-				errno, strerror(errno)));
-			ret_ok = false;
-			break;
-		}
-
-		DEBUG(11,("cups_async_callback: read namelen %u\n",
-			(unsigned int)namelen));
-
-		ret = sys_read(fd, &infolen, sizeof(infolen));
-		if (ret == 0) {
-			/* EOF */
-			break;
-		}
-		if (ret != sizeof(infolen)) {
-			DEBUG(10,("cups_async_callback: infolen read failed %s\n",
-				strerror(errno)));
-			ret_ok = false;
-			break;
-		}
+	ret_ok = recv_pcap_blob(frame, cb_args->pipe_fd, &pcap_blob);
+	if (!ret_ok) {


-- 
Samba Shared Repository


More information about the samba-cvs mailing list