Patch looks good.
minor comment, reading octal, '777' will be 511 decimal. (v>511)

Probably the between word should be omitted here.

Patch looks good.
minor comment, reading octal, '777' will be 511 decimal. (v>511)

Ahh, good point. I will revise those numbers accordingly...

In the test we could use @pytest.mark.parametrize(...) instead of calling a separate function repeatedly. The resulting logs of test runs would be better readable later on.

In the test we could use @pytest.mark.parametrize(...) instead of calling a separate function repeatedly. The resulting logs of test runs would be better readable later on.

I did think of that, but I personally think it is harder to read those big lumps of parameters. I can change it I suppose...

In the test we could use @pytest.mark.parametrize(...) instead of calling a separate function repeatedly. The resulting logs of test runs would be better readable later on.

I did think of that, but I personally think it is harder to read those big lumps of parameters. I can change it I suppose...

That was just a suggestion :), the code is well readable anyways. But with the parametrize, all the cases would be run separately, so one assert fail would not result in not running the rest of attributes' checks, as an added benefit.

@tbordaz, @mhonek - changes made please review...

strlen could be your first check after the endp != NULL, given it's probably the most likely to be bad.

Just a nit pick... :) Now we could just replace the call with the function's contents.

511 could be replaced by 0777 here, where the 0 implies octal number.

Superfluous space character after word be.

One more superfluous word between.

The upper bound 60 here is excluded, unlike in other log messages where we include the upper boundary number. Notice the > 59 in the condition, which should be > 60, I think.

Edit to my last comment: from the context it seems the condition is actually OK, just the log message should be probably changed to show between 0 and 59.

In the test we could use @pytest.mark.parametrize(...) instead of calling a separate function repeatedly. The resulting logs of test runs would be better readable later on.

I did think of that, but I personally think it is harder to read those big lumps of parameters. I can change it I suppose...

Parameters can be assigned with human readable IDs: https://docs.pytest.org/en/latest/example/parametrize.html (see "Different options for test IDs")

Edit to my last comment: from the context it seems the condition is actually OK, just the log message should be probably changed to show between 0 and 59.

Right the wording in the message is misleading, I'll get these things changed shortly...

Changes made please review

389-ds-base is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in 389-ds-base's github repository.

This pull request has been cloned to Github as issue and is available here:
- https://github.com/389ds/389-ds-base/issues/3453

If you want to continue to work on the PR, please navigate to the github issue,
download the patch from the attachments and file a new pull request.

Thank you for understanding. We apologize for all inconvenience.