From e75c36a9ce6930e200ac39e8dee6eb3dc89135e0 Mon Sep 17 00:00:00 2001 From: David Disseldorp Date: Tue, 24 May 2011 11:34:59 +0200 Subject: [PATCH 1/4] s3-printing: an empty cups printer list is treated as an error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit cups_async_callback() is called to receive new printcap data from a child process which requests the information from cupsd. Newly received printcap information is stored in a temporary printcap cache (tmp_pcap_cache). Once the child process closes the printcap IPC file descriptor, the system printcap cache is replaced with the newly populated tmp_pcap_cache, however this only occurs if tmp_pcap_cache is non null (has at least one printer). If the printcap cache is empty, which is the case when cups is not exporting any printers, the printcap cache is not replaced resulting in stale data. Signed-off-by: Günther Deschner --- source3/printing/print_cups.c | 23 +++++++++++++++++------ 1 files changed, 17 insertions(+), 6 deletions(-) diff --git a/source3/printing/print_cups.c b/source3/printing/print_cups.c index a8cc538..2b3b8ed 100644 --- a/source3/printing/print_cups.c +++ b/source3/printing/print_cups.c @@ -454,6 +454,7 @@ static void cups_async_callback(struct event_context *event_ctx, 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; DEBUG(5,("cups_async_callback: callback received for printer data. " "fd = %d\n", fd)); @@ -471,6 +472,7 @@ static void cups_async_callback(struct event_context *event_ctx, if (ret != sizeof(namelen)) { DEBUG(10,("cups_async_callback: namelen read failed %d %s\n", errno, strerror(errno))); + ret_ok = false; break; } @@ -485,6 +487,7 @@ static void cups_async_callback(struct event_context *event_ctx, if (ret != sizeof(infolen)) { DEBUG(10,("cups_async_callback: infolen read failed %s\n", strerror(errno))); + ret_ok = false; break; } @@ -494,6 +497,7 @@ static void cups_async_callback(struct event_context *event_ctx, if (namelen) { name = TALLOC_ARRAY(frame, char, namelen); if (!name) { + ret_ok = false; break; } ret = sys_read(fd, name, namelen); @@ -504,6 +508,7 @@ static void cups_async_callback(struct event_context *event_ctx, if (ret != namelen) { DEBUG(10,("cups_async_callback: name read failed %s\n", strerror(errno))); + ret_ok = false; break; } DEBUG(11,("cups_async_callback: read name %s\n", @@ -514,6 +519,7 @@ static void cups_async_callback(struct event_context *event_ctx, if (infolen) { info = TALLOC_ARRAY(frame, char, infolen); if (!info) { + ret_ok = false; break; } ret = sys_read(fd, info, infolen); @@ -524,6 +530,7 @@ static void cups_async_callback(struct event_context *event_ctx, if (ret != infolen) { DEBUG(10,("cups_async_callback: info read failed %s\n", strerror(errno))); + ret_ok = false; break; } DEBUG(11,("cups_async_callback: read info %s\n", @@ -533,14 +540,21 @@ static void cups_async_callback(struct event_context *event_ctx, } /* Add to our local pcap cache. */ - pcap_cache_add_specific(&tmp_pcap_cache, name, info); + ret_ok = pcap_cache_add_specific(&tmp_pcap_cache, name, info); TALLOC_FREE(name); TALLOC_FREE(info); + if (!ret_ok) { + DEBUG(0, ("failed to add to tmp pcap cache\n")); + break; + } } TALLOC_FREE(frame); - if (tmp_pcap_cache) { - /* We got a namelist, replace our local cache. */ + if (!ret_ok) { + DEBUG(0,("failed to read a new printer list\n")); + pcap_cache_destroy_specific(&tmp_pcap_cache); + } else { + /* We got a possibly empty namelist, replace our local cache. */ pcap_cache_destroy_specific(&local_pcap_copy); local_pcap_copy = tmp_pcap_cache; @@ -551,9 +565,6 @@ static void cups_async_callback(struct event_context *event_ctx, if (cb_args->post_cache_fill_fn) { cb_args->post_cache_fill_fn(); } - } else { - DEBUG(2,("cups_async_callback: failed to read a new " - "printer list\n")); } close(fd); TALLOC_FREE(cb_args); -- 1.7.4.4 From 52a37a560ab61bb9fe15816c4442a52ba0229cd1 Mon Sep 17 00:00:00 2001 From: David Disseldorp Date: Tue, 24 May 2011 11:41:27 +0200 Subject: [PATCH 2/4] idl: define printcap IPC message format MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Günther Deschner --- source3/Makefile.in | 3 +- source3/librpc/gen_ndr/ndr_printcap.c | 169 +++++++++++++++++++++++++++++++++ source3/librpc/gen_ndr/ndr_printcap.h | 14 +++ source3/librpc/gen_ndr/printcap.h | 21 ++++ source3/librpc/idl/printcap.idl | 17 ++++ 5 files changed, 223 insertions(+), 1 deletions(-) create mode 100644 source3/librpc/gen_ndr/ndr_printcap.c create mode 100644 source3/librpc/gen_ndr/ndr_printcap.h create mode 100644 source3/librpc/gen_ndr/printcap.h create mode 100644 source3/librpc/idl/printcap.idl diff --git a/source3/Makefile.in b/source3/Makefile.in index 82e108a..b7fdfce 100644 --- a/source3/Makefile.in +++ b/source3/Makefile.in @@ -1272,7 +1272,8 @@ samba3-idl:: srcdir="$(srcdir)" ../librpc/build_idl.sh ../librpc/idl/*.idl @PIDL_OUTPUTDIR="librpc/gen_ndr" PIDL_ARGS="$(PIDL_ARGS)" CPP="$(CPP)" PIDL="../pidl/pidl" \ srcdir="$(srcdir)" $(srcdir)/script/build_idl.sh \ - librpc/idl/messaging.idl librpc/idl/libnetapi.idl librpc/idl/notify.idl + librpc/idl/messaging.idl librpc/idl/libnetapi.idl librpc/idl/notify.idl \ + librpc/idl/printcap.idl ##################################################################### diff --git a/source3/librpc/gen_ndr/ndr_printcap.c b/source3/librpc/gen_ndr/ndr_printcap.c new file mode 100644 index 0000000..fdf55f1 --- /dev/null +++ b/source3/librpc/gen_ndr/ndr_printcap.c @@ -0,0 +1,169 @@ +/* parser auto-generated by pidl */ + +#include "includes.h" +#include "librpc/gen_ndr/ndr_printcap.h" + +static enum ndr_err_code ndr_push_pcap_printer(struct ndr_push *ndr, int ndr_flags, const struct pcap_printer *r) +{ + if (ndr_flags & NDR_SCALARS) { + NDR_CHECK(ndr_push_align(ndr, 4)); + NDR_CHECK(ndr_push_unique_ptr(ndr, r->name)); + NDR_CHECK(ndr_push_unique_ptr(ndr, r->info)); + } + if (ndr_flags & NDR_BUFFERS) { + if (r->name) { + NDR_CHECK(ndr_push_uint32(ndr, NDR_SCALARS, ndr_charset_length(r->name, CH_UTF8))); + NDR_CHECK(ndr_push_uint32(ndr, NDR_SCALARS, 0)); + NDR_CHECK(ndr_push_uint32(ndr, NDR_SCALARS, ndr_charset_length(r->name, CH_UTF8))); + NDR_CHECK(ndr_push_charset(ndr, NDR_SCALARS, r->name, ndr_charset_length(r->name, CH_UTF8), sizeof(uint8_t), CH_UTF8)); + } + if (r->info) { + NDR_CHECK(ndr_push_uint32(ndr, NDR_SCALARS, ndr_charset_length(r->info, CH_UTF8))); + NDR_CHECK(ndr_push_uint32(ndr, NDR_SCALARS, 0)); + NDR_CHECK(ndr_push_uint32(ndr, NDR_SCALARS, ndr_charset_length(r->info, CH_UTF8))); + NDR_CHECK(ndr_push_charset(ndr, NDR_SCALARS, r->info, ndr_charset_length(r->info, CH_UTF8), sizeof(uint8_t), CH_UTF8)); + } + } + return NDR_ERR_SUCCESS; +} + +static enum ndr_err_code ndr_pull_pcap_printer(struct ndr_pull *ndr, int ndr_flags, struct pcap_printer *r) +{ + uint32_t _ptr_name; + TALLOC_CTX *_mem_save_name_0; + uint32_t _ptr_info; + TALLOC_CTX *_mem_save_info_0; + if (ndr_flags & NDR_SCALARS) { + NDR_CHECK(ndr_pull_align(ndr, 4)); + NDR_CHECK(ndr_pull_generic_ptr(ndr, &_ptr_name)); + if (_ptr_name) { + NDR_PULL_ALLOC(ndr, r->name); + } else { + r->name = NULL; + } + NDR_CHECK(ndr_pull_generic_ptr(ndr, &_ptr_info)); + if (_ptr_info) { + NDR_PULL_ALLOC(ndr, r->info); + } else { + r->info = NULL; + } + } + if (ndr_flags & NDR_BUFFERS) { + if (r->name) { + _mem_save_name_0 = NDR_PULL_GET_MEM_CTX(ndr); + NDR_PULL_SET_MEM_CTX(ndr, r->name, 0); + NDR_CHECK(ndr_pull_array_size(ndr, &r->name)); + NDR_CHECK(ndr_pull_array_length(ndr, &r->name)); + if (ndr_get_array_length(ndr, &r->name) > ndr_get_array_size(ndr, &r->name)) { + return ndr_pull_error(ndr, NDR_ERR_ARRAY_SIZE, "Bad array size %u should exceed array length %u", ndr_get_array_size(ndr, &r->name), ndr_get_array_length(ndr, &r->name)); + } + NDR_CHECK(ndr_check_string_terminator(ndr, ndr_get_array_length(ndr, &r->name), sizeof(uint8_t))); + NDR_CHECK(ndr_pull_charset(ndr, NDR_SCALARS, &r->name, ndr_get_array_length(ndr, &r->name), sizeof(uint8_t), CH_UTF8)); + NDR_PULL_SET_MEM_CTX(ndr, _mem_save_name_0, 0); + } + if (r->info) { + _mem_save_info_0 = NDR_PULL_GET_MEM_CTX(ndr); + NDR_PULL_SET_MEM_CTX(ndr, r->info, 0); + NDR_CHECK(ndr_pull_array_size(ndr, &r->info)); + NDR_CHECK(ndr_pull_array_length(ndr, &r->info)); + if (ndr_get_array_length(ndr, &r->info) > ndr_get_array_size(ndr, &r->info)) { + return ndr_pull_error(ndr, NDR_ERR_ARRAY_SIZE, "Bad array size %u should exceed array length %u", ndr_get_array_size(ndr, &r->info), ndr_get_array_length(ndr, &r->info)); + } + NDR_CHECK(ndr_check_string_terminator(ndr, ndr_get_array_length(ndr, &r->info), sizeof(uint8_t))); + NDR_CHECK(ndr_pull_charset(ndr, NDR_SCALARS, &r->info, ndr_get_array_length(ndr, &r->info), sizeof(uint8_t), CH_UTF8)); + NDR_PULL_SET_MEM_CTX(ndr, _mem_save_info_0, 0); + } + } + return NDR_ERR_SUCCESS; +} + +_PUBLIC_ void ndr_print_pcap_printer(struct ndr_print *ndr, const char *name, const struct pcap_printer *r) +{ + ndr_print_struct(ndr, name, "pcap_printer"); + ndr->depth++; + ndr_print_ptr(ndr, "name", r->name); + ndr->depth++; + if (r->name) { + ndr_print_string(ndr, "name", r->name); + } + ndr->depth--; + ndr_print_ptr(ndr, "info", r->info); + ndr->depth++; + if (r->info) { + ndr_print_string(ndr, "info", r->info); + } + ndr->depth--; + ndr->depth--; +} + +_PUBLIC_ enum ndr_err_code ndr_push_pcap_data(struct ndr_push *ndr, int ndr_flags, const struct pcap_data *r) +{ + uint32_t cntr_printers_0; + if (ndr_flags & NDR_SCALARS) { + NDR_CHECK(ndr_push_uint32(ndr, NDR_SCALARS, r->count)); + NDR_CHECK(ndr_push_align(ndr, 4)); + NDR_CHECK(ndr_push_NTSTATUS(ndr, NDR_SCALARS, r->status)); + NDR_CHECK(ndr_push_uint32(ndr, NDR_SCALARS, r->count)); + for (cntr_printers_0 = 0; cntr_printers_0 < r->count; cntr_printers_0++) { + NDR_CHECK(ndr_push_pcap_printer(ndr, NDR_SCALARS, &r->printers[cntr_printers_0])); + } + } + if (ndr_flags & NDR_BUFFERS) { + for (cntr_printers_0 = 0; cntr_printers_0 < r->count; cntr_printers_0++) { + NDR_CHECK(ndr_push_pcap_printer(ndr, NDR_BUFFERS, &r->printers[cntr_printers_0])); + } + } + return NDR_ERR_SUCCESS; +} + +_PUBLIC_ enum ndr_err_code ndr_pull_pcap_data(struct ndr_pull *ndr, int ndr_flags, struct pcap_data *r) +{ + uint32_t cntr_printers_0; + TALLOC_CTX *_mem_save_printers_0; + if (ndr_flags & NDR_SCALARS) { + NDR_CHECK(ndr_pull_array_size(ndr, &r->printers)); + NDR_CHECK(ndr_pull_align(ndr, 4)); + NDR_CHECK(ndr_pull_NTSTATUS(ndr, NDR_SCALARS, &r->status)); + NDR_CHECK(ndr_pull_uint32(ndr, NDR_SCALARS, &r->count)); + NDR_PULL_ALLOC_N(ndr, r->printers, ndr_get_array_size(ndr, &r->printers)); + _mem_save_printers_0 = NDR_PULL_GET_MEM_CTX(ndr); + NDR_PULL_SET_MEM_CTX(ndr, r->printers, 0); + for (cntr_printers_0 = 0; cntr_printers_0 < r->count; cntr_printers_0++) { + NDR_CHECK(ndr_pull_pcap_printer(ndr, NDR_SCALARS, &r->printers[cntr_printers_0])); + } + NDR_PULL_SET_MEM_CTX(ndr, _mem_save_printers_0, 0); + if (r->printers) { + NDR_CHECK(ndr_check_array_size(ndr, (void*)&r->printers, r->count)); + } + } + if (ndr_flags & NDR_BUFFERS) { + _mem_save_printers_0 = NDR_PULL_GET_MEM_CTX(ndr); + NDR_PULL_SET_MEM_CTX(ndr, r->printers, 0); + for (cntr_printers_0 = 0; cntr_printers_0 < r->count; cntr_printers_0++) { + NDR_CHECK(ndr_pull_pcap_printer(ndr, NDR_BUFFERS, &r->printers[cntr_printers_0])); + } + NDR_PULL_SET_MEM_CTX(ndr, _mem_save_printers_0, 0); + } + return NDR_ERR_SUCCESS; +} + +_PUBLIC_ void ndr_print_pcap_data(struct ndr_print *ndr, const char *name, const struct pcap_data *r) +{ + uint32_t cntr_printers_0; + ndr_print_struct(ndr, name, "pcap_data"); + ndr->depth++; + ndr_print_NTSTATUS(ndr, "status", r->status); + ndr_print_uint32(ndr, "count", r->count); + ndr->print(ndr, "%s: ARRAY(%d)", "printers", (int)r->count); + ndr->depth++; + for (cntr_printers_0=0;cntr_printers_0count;cntr_printers_0++) { + char *idx_0=NULL; + if (asprintf(&idx_0, "[%d]", cntr_printers_0) != -1) { + ndr_print_pcap_printer(ndr, "printers", &r->printers[cntr_printers_0]); + free(idx_0); + } + } + ndr->depth--; + ndr->depth--; +} + diff --git a/source3/librpc/gen_ndr/ndr_printcap.h b/source3/librpc/gen_ndr/ndr_printcap.h new file mode 100644 index 0000000..37589da --- /dev/null +++ b/source3/librpc/gen_ndr/ndr_printcap.h @@ -0,0 +1,14 @@ +/* header auto-generated by pidl */ + +#include "librpc/ndr/libndr.h" +#include "librpc/gen_ndr/printcap.h" + +#ifndef _HEADER_NDR_printcap +#define _HEADER_NDR_printcap + +#define NDR_PRINTCAP_CALL_COUNT (0) +void ndr_print_pcap_printer(struct ndr_print *ndr, const char *name, const struct pcap_printer *r); +enum ndr_err_code ndr_push_pcap_data(struct ndr_push *ndr, int ndr_flags, const struct pcap_data *r); +enum ndr_err_code ndr_pull_pcap_data(struct ndr_pull *ndr, int ndr_flags, struct pcap_data *r); +void ndr_print_pcap_data(struct ndr_print *ndr, const char *name, const struct pcap_data *r); +#endif /* _HEADER_NDR_printcap */ diff --git a/source3/librpc/gen_ndr/printcap.h b/source3/librpc/gen_ndr/printcap.h new file mode 100644 index 0000000..c8d6350 --- /dev/null +++ b/source3/librpc/gen_ndr/printcap.h @@ -0,0 +1,21 @@ +/* header auto-generated by pidl */ + +#include + +#include "libcli/util/ntstatus.h" + +#ifndef _HEADER_printcap +#define _HEADER_printcap + +struct pcap_printer { + const char *name;/* [unique,charset(UTF8)] */ + const char *info;/* [unique,charset(UTF8)] */ +}; + +struct pcap_data { + NTSTATUS status; + uint32_t count; + struct pcap_printer *printers;/* [size_is(count)] */ +}/* [public] */; + +#endif /* _HEADER_printcap */ diff --git a/source3/librpc/idl/printcap.idl b/source3/librpc/idl/printcap.idl new file mode 100644 index 0000000..5ab380c --- /dev/null +++ b/source3/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; +} -- 1.7.4.4 From 06daf0fb185010fa69a33d656ad139a05302694c Mon Sep 17 00:00:00 2001 From: David Disseldorp Date: Tue, 24 May 2011 11:46:25 +0200 Subject: [PATCH 3/4] s3-printing: use printcap IDL for IPC MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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: Günther Deschner --- source3/Makefile.in | 2 +- source3/printing/print_cups.c | 231 +++++++++++++++++++---------------------- 2 files changed, 106 insertions(+), 127 deletions(-) diff --git a/source3/Makefile.in b/source3/Makefile.in index b7fdfce..ca5eaaf 100644 --- a/source3/Makefile.in +++ b/source3/Makefile.in @@ -755,7 +755,7 @@ SMBD_OBJ_BASE = $(PARAM_WITHOUT_REG_OBJ) $(SMBD_OBJ_SRV) $(LIBSMB_OBJ) \ 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_iprint.o librpc/gen_ndr/ndr_printcap.o PRINTBASE_OBJ = printing/notify.o printing/printing_db.o PRINTBACKEND_OBJ = printing/printing.o printing/nt_printing.o $(PRINTBASE_OBJ) diff --git a/source3/printing/print_cups.c b/source3/printing/print_cups.c index 2b3b8ed..5cea044 100644 --- a/source3/printing/print_cups.c +++ b/source3/printing/print_cups.c @@ -24,6 +24,7 @@ #include "includes.h" #include "printing.h" +#include "librpc/gen_ndr/ndr_printcap.h" #ifdef HAVE_CUPS #include @@ -106,46 +107,54 @@ 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; } - 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; - } + + 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; + } + + DEBUG(10, ("successfully recvd blob of len %ld\n", (int64_t)ret)); + return true; } 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; + struct pcap_printer *printer; http_t *http = NULL; /* HTTP connection to server */ ipp_t *request = NULL, /* IPP Request */ *response = NULL; /* IPP Response */ @@ -160,6 +169,11 @@ static bool cups_cache_reload_async(int fd) }; 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")); @@ -263,9 +277,20 @@ static bool cups_cache_reload_async(int fd) if (name == NULL) break; - if (!pcap_cache_add_specific(&tmp_pcap_cache, name, info)) { + if (pcap_data.count == 0) { + printer = talloc_array(frame, struct pcap_printer, 1); + } else { + printer = talloc_realloc(frame, pcap_data.printers, + struct pcap_printer, + pcap_data.count + 1); + } + if (printer == NULL) { goto out; } + pcap_data.printers = printer; + pcap_data.printers[pcap_data.count].name = name; + pcap_data.printers[pcap_data.count].info = info; + pcap_data.count++; } ippDelete(response); @@ -355,13 +380,24 @@ static bool cups_cache_reload_async(int fd) if (name == NULL) break; - if (!pcap_cache_add_specific(&tmp_pcap_cache, name, info)) { + if (pcap_data.count == 0) { + printer = talloc_array(frame, struct pcap_printer, 1); + } else { + printer = talloc_realloc(frame, pcap_data.printers, + struct pcap_printer, + pcap_data.count + 1); + } + if (printer == NULL) { goto 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 = True; - + ret = true; + pcap_data.status = NT_STATUS_OK; out: if (response) ippDelete(response); @@ -373,13 +409,14 @@ static bool cups_cache_reload_async(int fd) 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); + ndr_ret = ndr_push_struct_blob(&pcap_blob, frame, NULL, &pcap_data, + (ndr_push_flags_fn_t)ndr_push_pcap_data); + if (ndr_ret == NDR_ERR_SUCCESS) { + ret = send_pcap_blob(&pcap_blob, fd); + } else { + ret = false; } + TALLOC_FREE(frame); return ret; } @@ -452,104 +489,44 @@ 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) { + DEBUG(0,("failed to recv pcap blob\n")); + goto err_out; + } - DEBUG(11,("cups_async_callback: read infolen %u\n", - (unsigned int)infolen)); + ndr_ret = ndr_pull_struct_blob(&pcap_blob, frame, NULL, &pcap_data, + (ndr_pull_flags_fn_t)ndr_pull_pcap_data); + if (ndr_ret != NDR_ERR_SUCCESS) { + goto err_out; + } - if (namelen) { - name = TALLOC_ARRAY(frame, char, namelen); - if (!name) { - ret_ok = false; - break; - } - ret = sys_read(fd, name, namelen); - if (ret == 0) { - /* EOF */ - break; - } - if (ret != namelen) { - DEBUG(10,("cups_async_callback: name read failed %s\n", - strerror(errno))); - ret_ok = false; - break; - } - DEBUG(11,("cups_async_callback: read name %s\n", - name)); - } else { - name = NULL; - } - if (infolen) { - info = TALLOC_ARRAY(frame, char, infolen); - if (!info) { - ret_ok = false; - break; - } - ret = sys_read(fd, info, infolen); - if (ret == 0) { - /* EOF */ - break; - } - if (ret != infolen) { - DEBUG(10,("cups_async_callback: info read failed %s\n", - strerror(errno))); - ret_ok = false; - break; - } - DEBUG(11,("cups_async_callback: read info %s\n", - info)); - } else { - info = NULL; - } + if (!NT_STATUS_IS_OK(pcap_data.status)) { + DEBUG(0,("failed to retrieve printer list: %s\n", + nt_errstr(pcap_data.status))); + goto err_out; + } - /* Add to our local pcap cache. */ - ret_ok = pcap_cache_add_specific(&tmp_pcap_cache, name, info); - TALLOC_FREE(name); - TALLOC_FREE(info); + for (i = 0; i < pcap_data.count; i++) { + ret_ok = pcap_cache_add_specific(&tmp_pcap_cache, + pcap_data.printers[i].name, + pcap_data.printers[i].info); if (!ret_ok) { DEBUG(0, ("failed to add to tmp pcap cache\n")); break; } } - TALLOC_FREE(frame); if (!ret_ok) { DEBUG(0,("failed to read a new printer list\n")); pcap_cache_destroy_specific(&tmp_pcap_cache); @@ -566,7 +543,9 @@ static void cups_async_callback(struct event_context *event_ctx, cb_args->post_cache_fill_fn(); } } - close(fd); +err_out: + TALLOC_FREE(frame); + close(cb_args->pipe_fd); TALLOC_FREE(cb_args); TALLOC_FREE(cache_fd_event); } -- 1.7.4.4 From 75e7b56aa815642e5cc814181771893be47a5798 Mon Sep 17 00:00:00 2001 From: David Disseldorp Date: Tue, 24 May 2011 11:50:12 +0200 Subject: [PATCH 4/4] s3-printing: remove duplicate cups response processing code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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: Günther Deschner --- source3/printing/print_cups.c | 241 ++++++++++++++++------------------------- 1 files changed, 94 insertions(+), 147 deletions(-) diff --git a/source3/printing/print_cups.c b/source3/printing/print_cups.c index 5cea044..333d6f0 100644 --- a/source3/printing/print_cups.c +++ b/source3/printing/print_cups.c @@ -150,25 +150,105 @@ static bool recv_pcap_blob(TALLOC_CTX *mem_ctx, int fd, DATA_BLOB *pcap_blob) 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_data pcap_data; - struct pcap_printer *printer; 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; @@ -183,10 +263,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; } @@ -218,79 +294,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_data.count == 0) { - printer = talloc_array(frame, struct pcap_printer, 1); - } else { - printer = talloc_realloc(frame, pcap_data.printers, - struct pcap_printer, - pcap_data.count + 1); - } - if (printer == NULL) { - goto 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 = process_cups_printers_response(frame, response, &pcap_data); + if (!ret) { + DEBUG(0,("failed to process cups response\n")); + goto out; } ippDelete(response); @@ -321,82 +334,18 @@ 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_data.count == 0) { - printer = talloc_array(frame, struct pcap_printer, 1); - } else { - printer = talloc_realloc(frame, pcap_data.printers, - struct pcap_printer, - pcap_data.count + 1); - } - if (printer == NULL) { - goto 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 = 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) @@ -408,13 +357,11 @@ static bool cups_cache_reload_async(int fd) if (http) httpClose(http); - /* Send all the entries up the pipe. */ + ret = false; ndr_ret = ndr_push_struct_blob(&pcap_blob, frame, NULL, &pcap_data, (ndr_push_flags_fn_t)ndr_push_pcap_data); if (ndr_ret == NDR_ERR_SUCCESS) { ret = send_pcap_blob(&pcap_blob, fd); - } else { - ret = false; } TALLOC_FREE(frame); -- 1.7.4.4