On Fri, Aug 29, 2025 at 02:12:28PM -0500, Crystal Wood wrote:
> On Tue, 2025-08-12 at 13:21 -0400, Ivan Pravdin wrote:
> > Currently, user can only specify cgroup to the tracer's thread the
> > following ways:
> >
> > `-C[cgroup]`
> > `-C[=cgroup]`
> > `--cgroup[=cgroup]`
> >
> > If user tries to specify cgroup as `-C [cgroup]` or `--cgroup [cgroup]`,
> > the parser silently fails and rtla's cgroup is used for the tracer
> > threads.
> >
> > To make interface more user-friendly, allow user to specify cgroup in
> > the aforementioned way, i.e. `-C [cgroup]` and `--cgroup [cgroup]`
> >
> > Change documentation to reflect this user interface change.
>
> I know these are the semantics that --trace implements, but they're
> rather atypical... especially -C=group.
The new semantics allow for -C [group] which is the same as -t
[filename] that has been fixed for -t/--trace.
>
> > @@ -559,12 +559,17 @@ static struct osnoise_params
> > break;
> > case 'C':
> > params->cgroup = 1;
> > - if (!optarg) {
> > - /* will inherit this cgroup */
> > + if (optarg) {
> > + if (optarg[0] == '=') {
> > + /* skip the = */
> > + params->cgroup_name = &optarg[1];
> > + } else {
> > + params->cgroup_name = optarg;
> > + }
> > + } else if (optind < argc && argv[optind][0] != '-') {
> > + params->cgroup_name = argv[optind];
> > + } else {
> > params->cgroup_name = NULL;
> > - } else if (*optarg == '=') {
> > - /* skip the = */
> > - params->cgroup_name = ++optarg;
>
> If we're going to be consistently using these semantics, we should move
> this logic into a utility function rather than open-coding it
> everywhere.
Sure, I will move it into a utility function in the next version.
> Also, theoretically, shouldn't we be advancing optind for the case where
> that's consumed? Not that it matters much if we don't have positional
> arguments once the options begin, and if we did, then allowing
> "--arg optional-thing" would be ambiguous...
It does seem that we should advance optind when optional argument is
provided. Good catch. I will fix it in the next version.
> -Crystal
>
Ivan Pravdin
