Thank you for doing all the cleanup work!
Two small issues that I could still find:
On 11.09.2017 22:21, Thomas Helland wrote:
Based on Vladislav Egorovs work on the preprocessor, but split
out to a util functionality that should be universal. Setup, teardown,
memory handling and general layout is modeled around the hash_table
and the set, to make it familiar for everyone.
A notable change is that this implementation is always null terminated.
The rationale is that it will be less error-prone, as one might
access the buffer directly, thereby reading a non-terminated string.
Also, vsnprintf and friends prints the null-terminator.
V2: Address review feedback from Timothy and Grazvydas
- Fix MINGW preprocessor check
- Changed len from uint to int
- Make string argument const in append function
- Move to header and inline append function
- Add crimp_to_fit function for resizing buffer
V3: Move include of ralloc to string_buffer.h
V4: Use u_string.h for a cross-platform working vsnprintf
V5: Remember to cast to char * in crimp function
V6: Address review feedback from Nicolai
- Handle !str->buf in buffer_create
- Ensure va_end is always called in buffer_append_all
- Add overflow check in buffer_append_len
- Do not expose buffer_space_left, just remove it
- Clarify why a loop is used in vprintf, change to for-loop
- Add a va_copy to buffer_vprintf to fix failure to append arguments
when having to resize the buffer for vsnprintf.
[snip]
+bool
+_mesa_string_buffer_vprintf(struct _mesa_string_buffer *str,
+ const char *format, va_list args)
+{
+ /* We're looping two times to avoid duplicating code */
+ for (uint32_t i = 0; i < 2; i++) {
+ va_list arg_copy;
+ va_copy(arg_copy, args);
+ uint32_t space_left = str->capacity - str->length;
+
+ int32_t len = util_vsnprintf(str->buf + str->length,
+ space_left, format, arg_copy);
va_copy also needs to be matched with a va_end here.
+
+ /* Error in vsnprintf() or measured len overflows size_t */
+ if (unlikely(len < 0 || str->length + len + 1 < str->length))
+ return false;
+
+ /* There was enough space for the string; we're done */
+ if (len < space_left) {
+ str->length += len;
+ return true;
+ }
+
+ /* Not enough space, resize and retry */
+ ensure_capacity(str, str->length + len + 1);
+ }
+
+ return false;
+}
[snip]
+static inline void
+_mesa_string_buffer_crimp_to_fit(struct _mesa_string_buffer *str)
+{
+ str->capacity = str->length + 1;
+ str->buf = (char *) reralloc_array_size(str, str->buf,
+ sizeof(char), str->capacity);
What if the re-realloc fails?
I'd suggest:
char *crimped = (char *) reralloc_array_size(...);
if (!crimped)
return;
str->capacity = str->length + 1;
str->buf = crimped;
With those two issues addressed, patches 1&2 are also:
Reviewed-by: Nicolai Hähnle <[email protected]>
No need to re-send the series as far as I'm concerned.
+}
+
+bool
+_mesa_string_buffer_vprintf(struct _mesa_string_buffer *str,
+ const char *format, va_list args);
+
+bool
+_mesa_string_buffer_printf(struct _mesa_string_buffer *str,
+ const char *format, ...);
+
+#ifdef __cplusplus
+} /* extern "C" */
+#endif
+
+#endif /* _STRING_BUFFER_H */
--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
_______________________________________________
mesa-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
