On 1/11/2021 11:27 AM, Taylor Blau wrote: > On Mon, Jan 11, 2021 at 06:57:00AM -0500, Derrick Stolee wrote: >> Not that this is new to the current patch, but this patch made me >> wonder if we should initialize *pos = -1 in the case of a failure >> to find the position? A correct caller should not use the value >> if they are checking for the fail-to-find case properly. But, I >> could see someone making a mistake and having trouble diagnosing >> the problem because their position variable was initialized to >> zero or a previous successful case. > > *pos = -1 may be more confusing than clarifying since pos is unsigned. RIGHT. My bad. > It would be nice if there was a clear signal beyond returning a negative > value. I guess you could take a double pointer here which would allow > you to assign NULL, but that feels rather cumbersome as a means to catch > callers who failed to check the return value. > > It does raise the argument of whether or not we should allow the > program to continue at all if 'ret < 0' (i.e., 'offset_to_pack_pos()' > either 'die()'s or returns a usable uint32_t), but I'm OK with the > current behavior. I was thinking "*pos = -1" was a free way to "help" a developer who uses the API incorrectly, but it's _not_ free. Ignore me. Thanks, -Stolee
