Coding style improvements to ticket.c #153
Conversation
When we have a condition with a bunch of code under it, these can often be rewritten to invert the condition and exit which allows all the previously indented code to be un-indented. This can make the resulting code easier to follow. I haven't changed every single one of these in ticket.c, only the spots that I think makes it easier to see what's going on.
|
Rebased on #152 |
There was a problem hiding this comment.
There is something wrong with the patchset. I have 2 clusters and 1 arbitrator. Only one cluster is running booth from this PR. If so and ticket is granted to PR booth, then booth list displays wrong information (adds (null)) - ticket: apacheticket, leader: 192.168.63.103, expires: 2024-10-07 16:32:31(null). Witthout the PR everything works as expected ticket: apacheticket, leader: 192.168.63.104, expires: 2024-10-07 16:37:06. Not so sure how deeply the problem goes, but this should be fixed
* Skip all the counting at the beginning to decide how much memory to allocate, and instead allocate a string that should typically be large enough. However, GStrings can grow dynamically so it's not critical we get this completely correct. * Remove all the snprintf calls that have to keep track of how much they've already allocated. Instead, use g_string_append_printf or g_string_append as appropriate. * Simplify the pending_str block. Use asprintf to build up this string since we have to pass a static buffer to strftime. Then if we ever created a pending string, append and free it later. * Simplify the multiple_grant_warning_length check to not use a prefix decrement operator. We can just compare it to the number we actually care about instead. * Return a char * and free the GString internally so callers don't have to care about the implementation, and don't also return the length of the string. Callers can check that when they need to know it.
Good catch. Looks like it was just a matter of not unconditionally appending pending_str. I've updated the list_ticket patch to correct this. |
I've resisted doing these kinds of changes but there's several things about existing coding style (primarily, the lack of braces in a lot of places) that annoy me. And then once I started doing that, I found other things to do. This PR is mostly style stuff, though I also took the opportunity to simplify list_ticket.
I'm especially interested in a double check that handling the length of the string returned from list_ticket is correct.
Also - this builds on top of #152. The first four patches here are from that PR. Once that gets merged, I can rebase this one and they'll go away.