lib: Add STREAM_GETX functions

Currently when stream reads fail, for any reason, we assert.
While a *great* debugging tool, Asserting on production code
is not a good thing.  So this is the start of a conversion over
to a series of STREAM_GETX functions that do not assert and
allow the developer a way to program this gracefully and still
clean up.

Current code is something like this( taken from redistribute.c
because this is dead simple ):

	afi = stream_getc(client->ibuf);
	type = stream_getc(client->ibuf);
	instance = stream_getw(client->ibuf);

This code has several issues:

1) There is no failure mode for the stream read other than assert.
if afi fails to be read the code stops.
2) stream_getX functions cannot be converted to a failure mode
because it is impossible to tell a failure from good data
with this api.

So this new code will convert to this:

	STREAM_GETC(client->ibuf, afi);
	STREAM_GETC(client->ibuf, type);
	STREAM_GETW(client->ibuf, instance);

	....

stream_failure:
	return;

We've created a stream_getc2( which does not assert ),
but we need a way to allow clean failure mode handling.
This is done by macro'ing stream_getX2 functions with
the equivalent all uppercase STREAM_GETX functions that
include a goto.

Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
This commit is contained in:
Donald Sharp 2017-11-02 08:37:06 -04:00
parent 4df759fecf
commit 051cc28c8f
2 changed files with 123 additions and 6 deletions

View File

@ -65,12 +65,19 @@ DEFINE_MTYPE_STATIC(LIB, STREAM_FIFO, "Stream FIFO")
assert(ENDP_VALID(S, (S)->endp)); \
} while (0)
#define STREAM_BOUND_WARN(S, WHAT) \
do { \
zlog_warn("%s: Attempt to %s out of bounds", __func__, \
(WHAT)); \
STREAM_WARN_OFFSETS(S); \
assert(0); \
#define STREAM_BOUND_WARN(S, WHAT) \
do { \
zlog_warn("%s: Attempt to %s out of bounds", __func__, \
(WHAT)); \
STREAM_WARN_OFFSETS(S); \
assert(0); \
} while (0)
#define STREAM_BOUND_WARN2(S, WHAT) \
do { \
zlog_warn("%s: Attempt to %s out of bounds", __func__, \
(WHAT)); \
STREAM_WARN_OFFSETS(S); \
} while (0)
/* XXX: Deprecated macro: do not use */
@ -263,6 +270,21 @@ void stream_forward_endp(struct stream *s, size_t size)
}
/* Copy from stream to destination. */
inline bool stream_get2(void *dst, struct stream *s, size_t size)
{
STREAM_VERIFY_SANE(s);
if (STREAM_READABLE(s) < size) {
STREAM_BOUND_WARN2(s, "get");
return false;
}
memcpy(dst, s->data + s->getp, size);
s->getp += size;
return true;
}
void stream_get(void *dst, struct stream *s, size_t size)
{
STREAM_VERIFY_SANE(s);
@ -277,6 +299,19 @@ void stream_get(void *dst, struct stream *s, size_t size)
}
/* Get next character from the stream. */
inline bool stream_getc2(struct stream *s, u_char *byte)
{
STREAM_VERIFY_SANE(s);
if (STREAM_READABLE(s) < sizeof(u_char)) {
STREAM_BOUND_WARN2(s, "get char");
return false;
}
*byte = s->data[s->getp++];
return true;
}
u_char stream_getc(struct stream *s)
{
u_char c;
@ -309,6 +344,21 @@ u_char stream_getc_from(struct stream *s, size_t from)
return c;
}
inline bool stream_getw2(struct stream *s, uint16_t *word)
{
STREAM_VERIFY_SANE(s);
if (STREAM_READABLE(s) < sizeof(uint16_t)) {
STREAM_BOUND_WARN2(s, "get ");
return false;
}
*word = s->data[s->getp++] << 8;
*word |= s->data[s->getp++];
return true;
}
/* Get next word from the stream. */
u_int16_t stream_getw(struct stream *s)
{
@ -415,6 +465,24 @@ void stream_get_from(void *dst, struct stream *s, size_t from, size_t size)
memcpy(dst, s->data + from, size);
}
inline bool stream_getl2(struct stream *s, uint32_t *l)
{
STREAM_VERIFY_SANE(s);
if (STREAM_READABLE(s) < sizeof(uint32_t)) {
STREAM_BOUND_WARN2(s, "get long");
return false;
}
*l = (unsigned int)(s->data[s->getp++]) << 24;
*l |= s->data[s->getp++] << 16;
*l |= s->data[s->getp++] << 8;
*l |= s->data[s->getp++];
return true;
}
u_int32_t stream_getl(struct stream *s)
{
u_int32_t l;

View File

@ -181,14 +181,18 @@ extern int stream_put_prefix(struct stream *, struct prefix *);
extern int stream_put_labeled_prefix(struct stream *, struct prefix *,
mpls_label_t *);
extern void stream_get(void *, struct stream *, size_t);
extern bool stream_get2(void *data, struct stream *s, size_t size);
extern void stream_get_from(void *, struct stream *, size_t, size_t);
extern u_char stream_getc(struct stream *);
extern bool stream_getc2(struct stream *s, u_char *byte);
extern u_char stream_getc_from(struct stream *, size_t);
extern u_int16_t stream_getw(struct stream *);
extern bool stream_getw2(struct stream *s, uint16_t *word);
extern u_int16_t stream_getw_from(struct stream *, size_t);
extern u_int32_t stream_get3(struct stream *);
extern u_int32_t stream_get3_from(struct stream *, size_t);
extern u_int32_t stream_getl(struct stream *);
extern bool stream_getl2(struct stream *s, uint32_t *l);
extern u_int32_t stream_getl_from(struct stream *, size_t);
extern uint64_t stream_getq(struct stream *);
extern uint64_t stream_getq_from(struct stream *, size_t);
@ -257,4 +261,49 @@ static inline uint8_t *ptr_get_be32(uint8_t *ptr, uint32_t *out)
return ptr + 4;
}
/*
* so Normal stream_getX functions assert. Which is anathema
* to keeping a daemon up and running when something goes south
* Provide a stream_getX2 functions that do not assert.
* In addition provide these macro's that upon failure
* goto stream_failure. This is modeled upon some NL_XX
* macros in the linux kernel.
*
* This change allows for proper memory freeing
* after we've detected an error.
*
* In the future we will be removing the assert in
* the stream functions but we need a transition
* plan.
*/
#define STREAM_GETC(S, P) \
do { \
uint8_t _pval; \
if (!stream_getc2((S), &_pval)) \
goto stream_failure; \
(P) = _pval; \
} while (0)
#define STREAM_GETW(S, P) \
do { \
uint16_t _pval; \
if (!stream_getw2((S), &_pval)) \
goto stream_failure; \
(P) = _pval; \
} while (0)
#define STREAM_GETL(S, P) \
do { \
uint32_t _pval; \
if (!stream_getl2((S), &_pval)) \
goto stream_failure; \
(P) = _pval; \
} while (0)
#define STREAM_GET(P, STR, SIZE) \
do { \
if (!stream_get2((P), (STR), (SIZE))) \
goto stream_failure; \
} while (0)
#endif /* _ZEBRA_STREAM_H */