lib/posix-process: Fix off-by-one error in tid check #1377
Conversation
Fix an off-by-one error in tid2pthread() that returns failure when the passed tid is the maximum allowed value. Show error message when CONFIG_LIBPOSIX_PROCESS_MAX_PID is reached. Signed-off-by: Michalis Pappas <michalis@unikraft.io>
github-actions
bot
added
area/lib
There was a problem hiding this comment.
Looks good to me.
Reviewed-by: Delia Pavel delia_maria.pavel@stud.acs.upb.ro
| @@ -110,6 +110,8 @@ static inline pid_t find_free_tid(void) | |||
| } | |||
| if (found == TIDMAP_SIZE) { | |||
| /* no free PID */ | |||
| uk_pr_err("Could not allocate TID: Out of TIDs (configured max: %d)\n", | |||
There was a problem hiding this comment.
Not much related, but I think we may move the second if that is checking if (found == TIDMAP_SIZE) inside the first one.
If we have found something with the first search, there is no need to check the condition if (found == TIDMAP_SIZE) two times.
There was a problem hiding this comment.
Hi @RaduNichita, yes, I added that message here while preserving the existing logic, but I think you're right. git blame shows that this pattern originates since inception of process.c, and until a1c73be both conditionals checked against CONFIG_LIBPOSIX_PROCESS_MAX_PID. Before making any changes I would like to check on the reasoning with @skuenzer who is the original author, but given that he's away this week I would prefer if we considered that change on a follow-up PR.
There was a problem hiding this comment.
Let's merge this and discuss with @skuenzer based on our discussion. Thanks @michpappas.
Reviewed-by: Radu Nichita radunichita99@gmail.com
Fix an off-by-one error in tid2pthread() that returns failure when the passed tid is the maximum allowed value. Show error message when CONFIG_LIBPOSIX_PROCESS_MAX_PID is reached. Signed-off-by: Michalis Pappas <michalis@unikraft.io> Reviewed-by: Delia Pavel <delia_maria.pavel@stud.acs.upb.ro> Reviewed-by: Radu Nichita <radunichita99@gmail.com> Approved-by: Razvan Deaconescu <razvand@unikraft.io> GitHub-Closes: #1377
unikraft-bot
added
ci/merged
Prerequisite checklist
checkpatch.ukon your commit series before opening this PR;Base target
Additional configuration
Description of changes
Fix an off-by-one error in tid2pthread() that returns failure when the passed tid is the maximum allowed value.
Show error message when CONFIG_LIBPOSIX_PROCESS_MAX_PID is reached.