This is an automated email from the ASF dual-hosted git repository.
astitcher pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/qpid-proton.git
commit 97fcd648fd826a68b3e0d681d054c11031e6b06a
Author: Andrew Stitcher <astitc...@apache.org>
AuthorDate: Wed Jul 5 23:38:46 2023 -0400
PROTON-2752: Make more use of pn_string_t instead of char*
---
c/src/core/emitters.h | 16 ++++-----
c/src/core/message.c | 14 ++++----
c/src/core/object/string.c | 13 +++++--
c/src/core/object_private.h | 1 +
c/src/core/transport.c | 72 ++++++++++++++++++-------------------
c/src/core/util.h | 4 ---
c/src/sasl/sasl-internal.h | 4 +--
c/src/sasl/sasl.c | 31 ++++++++--------
c/tools/codec-generator/generate.py | 4 +--
9 files changed, 82 insertions(+), 77 deletions(-)
diff --git a/c/src/core/emitters.h b/c/src/core/emitters.h
index a9399ba5c..46cec39db 100644
--- a/c/src/core/emitters.h
+++ b/c/src/core/emitters.h
@@ -414,23 +414,19 @@ static inline void emit_symbol_bytes(pni_emitter_t*
emitter, pni_compound_contex
compound->count++;
}
-static inline void emit_symbol(pni_emitter_t* emitter, pni_compound_context*
compound, const char* symbol) {
- // FIXME: Yuck we need to use strlen to find the end of the string - would
be better to take pn_bytes_t
- if (symbol == NULL) {
+static inline void emit_symbol(pni_emitter_t* emitter, pni_compound_context*
compound, pn_bytes_t bytes) {
+ if (bytes.start == NULL) {
emit_null(emitter, compound);
} else {
- size_t size = strlen(symbol);
- emit_symbol_bytes(emitter, compound, (pn_bytes_t){.size = size, .start =
symbol});
+ emit_symbol_bytes(emitter, compound, bytes);
}
}
-static inline void emit_string(pni_emitter_t* emitter, pni_compound_context*
compound, const char* string) {
- // FIXME: Yuck we need to use strlen to find the end of the string - would
be better to take pn_bytes_t
- if (string == NULL) {
+static inline void emit_string(pni_emitter_t* emitter, pni_compound_context*
compound, pn_bytes_t bytes) {
+ if (bytes.start == NULL) {
emit_null(emitter, compound);
} else {
- size_t size = strlen(string);
- emit_string_bytes(emitter, compound, (pn_bytes_t){.size = size, .start =
string});
+ emit_string_bytes(emitter, compound, bytes);
}
}
diff --git a/c/src/core/message.c b/c/src/core/message.c
index dd12a4207..b4def0b14 100644
--- a/c/src/core/message.c
+++ b/c/src/core/message.c
@@ -856,22 +856,22 @@ int pn_message_encode(pn_message_t *msg, char *bytes,
size_t *isize)
last_size = pn_amqp_encode_bytes_DLEazSSSassQtQtSQISe(bytes, remaining,
PROPERTIES,
&id,
pn_string_size(msg->user_id), pn_string_get(msg->user_id),
- pn_string_get(msg->address),
- pn_string_get(msg->subject),
- pn_string_get(msg->reply_to),
+ pn_string_bytes(msg->address),
+ pn_string_bytes(msg->subject),
+ pn_string_bytes(msg->reply_to),
&correlation_id,
- pn_string_get(msg->content_type),
- pn_string_get(msg->content_encoding),
+ pn_string_bytes(msg->content_type),
+ pn_string_bytes(msg->content_encoding),
(bool)msg->expiry_time, msg->expiry_time,
(bool)msg->creation_time, msg->creation_time,
- pn_string_get(msg->group_id),
+ pn_string_bytes(msg->group_id),
/*
* As a heuristic, null out group_sequence if there is no
group_id and
* group_sequence is 0. In this case it is extremely
unlikely we want
* group semantics
*/
(bool)pn_string_get(msg->group_id) ||
(bool)msg->group_sequence , msg->group_sequence,
- pn_string_get(msg->reply_to_group_id));
+ pn_string_bytes(msg->reply_to_group_id));
if (last_size > remaining) return PN_OVERFLOW;
remaining -= last_size;
diff --git a/c/src/core/object/string.c b/c/src/core/object/string.c
index efcb642f3..e773226fc 100644
--- a/c/src/core/object/string.c
+++ b/c/src/core/object/string.c
@@ -36,8 +36,8 @@
struct pn_string_t {
char *bytes;
- ssize_t size; // PNI_NULL_SIZE (-1) means null
- size_t capacity;
+ int32_t size; // PNI_NULL_SIZE (-1) means null
+ uint32_t capacity;
};
static void pn_string_finalize(void *object)
@@ -136,6 +136,15 @@ size_t pn_string_size(pn_string_t *string)
}
}
+pn_bytes_t pn_string_bytes(pn_string_t *string)
+{
+ if (!string || string->size == PNI_NULL_SIZE) {
+ return (pn_bytes_t){0, NULL};
+ } else {
+ return (pn_bytes_t){string->size, string->bytes};
+ }
+}
+
int pn_string_set(pn_string_t *string, const char *bytes)
{
return pn_string_setn(string, bytes, bytes ? strlen(bytes) : 0);
diff --git a/c/src/core/object_private.h b/c/src/core/object_private.h
index 12a784506..c6864625e 100644
--- a/c/src/core/object_private.h
+++ b/c/src/core/object_private.h
@@ -177,6 +177,7 @@ PN_EXTERN void *pn_hash_value(pn_hash_t *hash, pn_handle_t
entry);
PN_EXTERN pn_string_t *pn_string(const char *bytes);
PN_EXTERN const char *pn_string_get(pn_string_t *string);
+pn_bytes_t pn_string_bytes(pn_string_t *string);
PN_EXTERN pn_string_t *pn_stringn(const char *bytes, size_t n);
PN_EXTERN size_t pn_string_size(pn_string_t *string);
PN_EXTERN int pn_string_set(pn_string_t *string, const char *bytes);
diff --git a/c/src/core/transport.c b/c/src/core/transport.c
index 8e3fe2373..5dd37e43e 100644
--- a/c/src/core/transport.c
+++ b/c/src/core/transport.c
@@ -951,17 +951,17 @@ static int pni_post_close(pn_transport_t *transport,
pn_condition_t *cond)
if (!cond && transport->connection) {
cond = pn_connection_condition(transport->connection);
}
- const char *condition = NULL;
- const char *description = NULL;
+ pn_string_t *condition = NULL;
+ pn_string_t *description = NULL;
pn_data_t *info = NULL;
if (pn_condition_is_set(cond)) {
- condition = pn_condition_get_name(cond);
- description = pn_condition_get_description(cond);
+ condition = cond->name;
+ description = cond->description;
info = pn_condition_info(cond);
}
/* "DL[?DL[sSC]]" */
pn_bytes_t buf = pn_amqp_encode_DLEQDLEsSCee(&transport->scratch_space,
CLOSE,
- (bool) condition, ERROR, condition, description, info);
+ (bool) condition, ERROR, pn_string_bytes(condition),
pn_string_bytes(description), info);
return pn_framing_send_amqp(transport, 0, buf);
}
@@ -1193,16 +1193,16 @@ static pn_distribution_mode_t symbol2dist_mode(const
pn_bytes_t symbol)
return PN_DIST_MODE_UNSPECIFIED;
}
-static const char *dist_mode2symbol(const pn_distribution_mode_t mode)
+static pn_bytes_t dist_mode2symbol(const pn_distribution_mode_t mode)
{
switch (mode)
{
case PN_DIST_MODE_COPY:
- return "copy";
+ return PN_BYTES_LITERAL(copy);
case PN_DIST_MODE_MOVE:
- return "move";
+ return PN_BYTES_LITERAL(move);
default:
- return NULL;
+ return (pn_bytes_t){0, NULL};
}
}
@@ -1868,12 +1868,13 @@ static int pni_process_conn_setup(pn_transport_t
*transport, pn_endpoint_t *endp
? (transport->local_idle_timeout/2)
: 0;
pn_connection_t *connection = (pn_connection_t *) endpoint;
- const char *cid = pn_string_get(connection->container);
+ pn_bytes_t cid = pn_string_bytes(connection->container);
+ if (cid.start==NULL) cid = (pn_bytes_t){.size=0, .start=""};
pni_calculate_channel_max(transport);
/* "DL[SS?I?H?InnMMC]" */
pn_bytes_t buf =
pn_amqp_encode_DLESSQIQHQInnMMCe(&transport->scratch_space, OPEN,
- cid ? cid : "",
- pn_string_get(connection->hostname),
+ cid,
+ pn_string_bytes(connection->hostname),
// TODO: This is messy, because we also have to
allow local_max_frame_ to be 0 to mean unlimited
// otherwise flow control goes wrong
transport->local_max_frame!=0 &&
transport->local_max_frame!=OPEN_MAX_FRAME_SIZE_DEFAULT,
@@ -1973,22 +1974,22 @@ static int pni_process_ssn_setup(pn_transport_t
*transport, pn_endpoint_t *endpo
return 0;
}
-static const char *expiry_symbol(pn_terminus_t* terminus)
+static pn_bytes_t expiry_symbol(pn_terminus_t* terminus)
{
- if (!terminus->has_expiry_policy) return NULL;
+ if (!terminus->has_expiry_policy) return (pn_bytes_t){0, NULL};
switch (terminus->expiry_policy)
{
case PN_EXPIRE_WITH_LINK:
- return "link-detach";
+ return PN_BYTES_LITERAL(link-detach);
case PN_EXPIRE_WITH_SESSION:
- return "session-end";
+ return PN_BYTES_LITERAL(session-end);
case PN_EXPIRE_WITH_CONNECTION:
- return "connection-close";
+ return PN_BYTES_LITERAL(connection-close);
case PN_EXPIRE_NEVER:
- return "never";
+ return PN_BYTES_LITERAL(never);
}
- return NULL;
+ return (pn_bytes_t){0, NULL};
}
static int pni_map_local_handle(pn_link_t *link) {
@@ -2020,15 +2021,14 @@ static int pni_process_link_setup(pn_transport_t
*transport, pn_endpoint_t *endp
}
const pn_distribution_mode_t dist_mode = (pn_distribution_mode_t)
link->source.distribution_mode;
if (link->target.type == PN_COORDINATOR) {
- /* "DL[SIoBB?DL[SIsIoC?sCnCC]DL[C]nnI]" */
pn_bytes_t buf =
pn_amqp_encode_DLESIoBBQDLESIsIoCQsCnCCeDLECennIe(&transport->scratch_space,
ATTACH,
- pn_string_get(link->name),
+ pn_string_bytes(link->name),
state->local_handle,
endpoint->type == RECEIVER,
link->snd_settle_mode,
link->rcv_settle_mode,
(bool) link->source.type, SOURCE,
- pn_string_get(link->source.address),
+ pn_string_bytes(link->source.address),
link->source.durability,
expiry_symbol(&link->source),
link->source.timeout,
@@ -2045,14 +2045,14 @@ static int pni_process_link_setup(pn_transport_t
*transport, pn_endpoint_t *endp
} else {
/* "DL[SIoBB?DL[SIsIoC?sCnMM]?DL[SIsIoCM]nnILnnC]" */
pn_bytes_t buf =
pn_amqp_encode_DLESIoBBQDLESIsIoCQsCnMMeQDLESIsIoCMennILnnCe(&transport->scratch_space,
ATTACH,
- pn_string_get(link->name),
+ pn_string_bytes(link->name),
state->local_handle,
endpoint->type == RECEIVER,
link->snd_settle_mode,
link->rcv_settle_mode,
(bool) link->source.type, SOURCE,
- pn_string_get(link->source.address),
+ pn_string_bytes(link->source.address),
link->source.durability,
expiry_symbol(&link->source),
link->source.timeout,
@@ -2064,7 +2064,7 @@ static int pni_process_link_setup(pn_transport_t
*transport, pn_endpoint_t *endp
link->source.capabilities,
(bool) link->target.type, TARGET,
- pn_string_get(link->target.address),
+ pn_string_bytes(link->target.address),
link->target.durability,
expiry_symbol(&link->target),
link->target.timeout,
@@ -2391,20 +2391,20 @@ static int pni_process_link_teardown(pn_transport_t
*transport, pn_endpoint_t *e
(int16_t) ssn_state->remote_channel != -2 &&
!transport->close_rcvd) return 0;
- const char *name = NULL;
- const char *description = NULL;
+ pn_string_t *name = NULL;
+ pn_string_t *description = NULL;
pn_data_t *info = NULL;
if (pn_condition_is_set(&endpoint->condition)) {
- name = pn_condition_get_name(&endpoint->condition);
- description = pn_condition_get_description(&endpoint->condition);
+ name = endpoint->condition.name;
+ description = endpoint->condition.description;
info = pn_condition_info(&endpoint->condition);
}
/* "DL[I?o?DL[sSC]]" */
pn_bytes_t buf =
pn_amqp_encode_DLEIQoQDLEsSCee(&transport->scratch_space, DETACH,
state->local_handle,
!link->detached, !link->detached,
- (bool)name, ERROR, name,
description, info);
+ (bool)name, ERROR,
pn_string_bytes(name), pn_string_bytes(description), info);
int err = pn_framing_send_amqp(transport, ssn_state->local_channel, buf);
if (err) return err;
pni_unmap_local_handle(link);
@@ -2467,18 +2467,18 @@ static int pni_process_ssn_teardown(pn_transport_t
*transport, pn_endpoint_t *en
return 0;
}
- const char *name = NULL;
- const char *description = NULL;
+ pn_string_t *name = NULL;
+ pn_string_t *description = NULL;
pn_data_t *info = NULL;
if (pn_condition_is_set(&endpoint->condition)) {
- name = pn_condition_get_name(&endpoint->condition);
- description = pn_condition_get_description(&endpoint->condition);
+ name = endpoint->condition.name;
+ description = endpoint->condition.description;
info = pn_condition_info(&endpoint->condition);
}
/* "DL[?DL[sSC]]" */
pn_bytes_t buf = pn_amqp_encode_DLEQDLEsSCee(&transport->scratch_space,
END,
- (bool) name, ERROR, name, description, info);
+ (bool) name, ERROR, pn_string_bytes(name),
pn_string_bytes(description), info);
int err = pn_framing_send_amqp(transport, state->local_channel, buf);
if (err) return err;
pni_unmap_local_channel(session);
@@ -2554,7 +2554,7 @@ static void pn_error_amqp(pn_transport_t* transport,
unsigned int layer)
if (!transport->close_sent) {
if (!transport->open_sent) {
/* "DL[S]" */
- pn_bytes_t buf = pn_amqp_encode_DLESe(&transport->scratch_space, OPEN,
"");
+ pn_bytes_t buf = pn_amqp_encode_DLESe(&transport->scratch_space, OPEN,
(pn_bytes_t){.size=0, .start=""});
pn_framing_send_amqp(transport, 0, buf);
}
diff --git a/c/src/core/util.h b/c/src/core/util.h
index ceb70720f..d55d7b898 100644
--- a/c/src/core/util.h
+++ b/c/src/core/util.h
@@ -51,10 +51,6 @@ static inline bool pn_bytes_equal(const pn_bytes_t a, const
pn_bytes_t b) {
return (a.size == b.size && !memcmp(a.start, b.start, a.size));
}
-static inline pn_bytes_t pn_string_bytes(struct pn_string_t *s) {
- return pn_bytes(pn_string_size(s), pn_string_get(s));
-}
-
static inline pn_bytes_t pn_bytes_dup(pn_bytes_t in) {
if (in.size) {
char* rbytes = malloc(in.size);
diff --git a/c/src/sasl/sasl-internal.h b/c/src/sasl/sasl-internal.h
index 376854e0e..ae183361e 100644
--- a/c/src/sasl/sasl-internal.h
+++ b/c/src/sasl/sasl-internal.h
@@ -41,13 +41,13 @@ void pni_sasl_set_external_security(pn_transport_t
*transport, int ssf, const ch
struct pni_sasl_t {
void *impl_context;
const pnx_sasl_implementation* impl;
- char *selected_mechanism;
+ pn_string_t *selected_mechanism;
char *included_mechanisms;
const char *username;
const char *authzid;
char *password;
const char *remote_fqdn;
- char *local_fqdn;
+ pn_string_t *local_fqdn;
char *external_auth;
int external_ssf;
size_t max_encrypt_size;
diff --git a/c/src/sasl/sasl.c b/c/src/sasl/sasl.c
index 0dff90a6a..517a73dc4 100644
--- a/c/src/sasl/sasl.c
+++ b/c/src/sasl/sasl.c
@@ -136,7 +136,7 @@ const char *pnx_sasl_get_remote_fqdn(pn_transport_t
*transport)
const char *pnx_sasl_get_selected_mechanism(pn_transport_t *transport)
{
- return transport->sasl ? transport->sasl->selected_mechanism : NULL;
+ return transport->sasl ? pn_string_get(transport->sasl->selected_mechanism)
: NULL;
}
void pnx_sasl_set_bytes_out(pn_transport_t *transport, pn_bytes_t bytes)
@@ -149,7 +149,7 @@ void pnx_sasl_set_bytes_out(pn_transport_t *transport,
pn_bytes_t bytes)
void pnx_sasl_set_selected_mechanism(pn_transport_t *transport, const char
*mechanism)
{
if (transport->sasl) {
- transport->sasl->selected_mechanism = pn_strdup(mechanism);
+ transport->sasl->selected_mechanism = pn_string(mechanism);
}
}
@@ -163,10 +163,10 @@ void pnx_sasl_set_succeeded(pn_transport_t *transport,
const char *username, co
if (authzid) {
PN_LOG(&transport->logger, PN_SUBSYSTEM_SASL, PN_LEVEL_INFO,
"Authenticated user: %s for %s with mechanism %s",
- username, authzid, transport->sasl->selected_mechanism);
+ username, authzid,
pn_string_get(transport->sasl->selected_mechanism));
} else {
PN_LOG(&transport->logger, PN_SUBSYSTEM_SASL, PN_LEVEL_INFO,
"Authenticated user: %s with mechanism %s",
- username, transport->sasl->selected_mechanism);
+ username, pn_string_get(transport->sasl->selected_mechanism));
}
}
}
@@ -491,7 +491,7 @@ static void pni_post_sasl_frame(pn_transport_t *transport)
switch (desired_state) {
case SASL_POSTED_INIT: {
/* GENERATE_CODEC_CODE: "DL[szS]" */
- pn_bytes_t buf = pn_amqp_encode_DLEszSe(&transport->scratch_space,
SASL_INIT, sasl->selected_mechanism, out.size, out.start, sasl->local_fqdn);
+ pn_bytes_t buf = pn_amqp_encode_DLEszSe(&transport->scratch_space,
SASL_INIT, pn_string_bytes(sasl->selected_mechanism), out.size, out.start,
pn_string_bytes(sasl->local_fqdn));
pn_framing_send_sasl(transport, buf);
pni_emit(transport);
break;
@@ -541,8 +541,9 @@ static void pni_post_sasl_frame(pn_transport_t *transport)
pn_framing_send_sasl(transport, buf);
pni_emit(transport);
if (sasl->outcome!=PN_SASL_OK) {
+ const char *selected_mechanism = transport->sasl->selected_mechanism ?
pn_string_get(transport->sasl->selected_mechanism) : NULL;
pn_do_error(transport, "amqp:unauthorized-access", "Failed to
authenticate client [mech=%s]",
- transport->sasl->selected_mechanism ?
transport->sasl->selected_mechanism : "none");
+ selected_mechanism ? selected_mechanism : "none");
desired_state = SASL_ERROR;
}
break;
@@ -553,11 +554,13 @@ static void pni_post_sasl_frame(pn_transport_t *transport)
continue;
}
break;
- case SASL_RECVED_FAILURE:
+ case SASL_RECVED_FAILURE: {
+ const char *selected_mechanism = transport->sasl->selected_mechanism ?
pn_string_get(transport->sasl->selected_mechanism) : NULL;
pn_do_error(transport, "amqp:unauthorized-access", "Authentication
failed [mech=%s]",
- transport->sasl->selected_mechanism ?
transport->sasl->selected_mechanism : "none");
+ selected_mechanism ? selected_mechanism : "none");
desired_state = SASL_ERROR;
break;
+ }
case SASL_ERROR:
break;
case SASL_NONE:
@@ -793,11 +796,11 @@ void pn_sasl_free(pn_transport_t *transport)
if (transport) {
pni_sasl_t *sasl = transport->sasl;
if (sasl) {
- free(sasl->selected_mechanism);
+ pn_free(sasl->selected_mechanism);
free(sasl->included_mechanisms);
free(sasl->password);
free(sasl->external_auth);
- free(sasl->local_fqdn);
+ pn_free(sasl->local_fqdn);
if (sasl->impl_context) {
pni_sasl_impl_free(transport);
@@ -818,7 +821,7 @@ void pni_sasl_set_remote_hostname(pn_transport_t *
transport, const char * fqdn)
void pnx_sasl_set_local_hostname(pn_transport_t * transport, const char * fqdn)
{
pni_sasl_t *sasl = transport->sasl;
- sasl->local_fqdn = pn_strdup(fqdn);
+ sasl->local_fqdn = pn_string(fqdn);
}
void pni_sasl_set_user_password(pn_transport_t *transport, const char *user,
const char *authzid, const char *password)
@@ -853,7 +856,7 @@ const char *pn_sasl_get_authorization(pn_sasl_t *sasl0)
const char *pn_sasl_get_mech(pn_sasl_t *sasl0)
{
pni_sasl_t *sasl = get_sasl_internal(sasl0);
- return sasl->selected_mechanism;
+ return pn_string_get(sasl->selected_mechanism);
}
void pn_sasl_allowed_mechs(pn_sasl_t *sasl0, const char *mechs)
@@ -905,7 +908,7 @@ int pn_do_init(pn_transport_t *transport, uint8_t
frame_type, uint16_t channel,
pn_bytes_t recv;
pn_amqp_decode_DqEsze(payload, &mech, &recv);
- sasl->selected_mechanism = pn_strndup(mech.start, mech.size);
+ sasl->selected_mechanism = pn_stringn(mech.start, mech.size);
// We need to filter out a supplied mech in in the inclusion list
// as the client could have used a mech that we support, but that
@@ -915,7 +918,7 @@ int pn_do_init(pn_transport_t *transport, uint8_t
frame_type, uint16_t channel,
sasl->outcome = PN_SASL_AUTH;
pnx_sasl_set_desired_state(transport, SASL_POSTED_OUTCOME);
} else {
- pni_sasl_impl_process_init(transport, sasl->selected_mechanism, &recv);
+ pni_sasl_impl_process_init(transport,
pn_string_get(sasl->selected_mechanism), &recv);
}
return 0;
diff --git a/c/tools/codec-generator/generate.py
b/c/tools/codec-generator/generate.py
index e9ec2445c..260edf80f 100644
--- a/c/tools/codec-generator/generate.py
+++ b/c/tools/codec-generator/generate.py
@@ -304,9 +304,9 @@ def parse_item(format: str) -> Tuple[ASTNode, str]:
elif format.startswith('.'):
return NullNode('anything'), format[1:]
elif format.startswith('s'):
- return ASTNode('symbol', ['const char*'],
consume_types=['pn_bytes_t*']), format[1:]
+ return ASTNode('symbol', ['pn_bytes_t'],
consume_types=['pn_bytes_t*']), format[1:]
elif format.startswith('S'):
- return ASTNode('string', ['const char*'],
consume_types=['pn_bytes_t*']), format[1:]
+ return ASTNode('string', ['pn_bytes_t'],
consume_types=['pn_bytes_t*']), format[1:]
elif format.startswith('C'):
return ASTNode('copy', ['pn_data_t*'], consume_types=['pn_data_t*']),
format[1:]
elif format.startswith('I'):
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@qpid.apache.org
For additional commands, e-mail: commits-h...@qpid.apache.org
