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 <dd...@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 <a...@samba.org> Autobuild-User: Andreas Schneider <a...@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 <dd...@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 <a...@samba.org> (cherry picked from commit 52845c1054941e697143940b94a0792f4d4e07c5) commit e7b59b0b563db801edf12083003f95576b7b6df3 Author: David Disseldorp <dd...@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 <a...@samba.org> (cherry picked from commit d6cb4feae1eab22d63b42eb5c480578fb1ee99bf) commit ac311ed7b615ebede4c3e1d44768ae11880665fc Author: David Disseldorp <dd...@suse.de> Date: Mon Mar 7 15:32:02 2011 +0100 idl: define printcap IPC message format Signed-off-by: Andreas Schneider <a...@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