public inbox for ltp@lists.linux.it
 help / color / mirror / Atom feed
From: Richard Palethorpe <rpalethorpe@suse.de>
To: ltp@lists.linux.it
Subject: [LTP] [PATCH v4 3/3] API: Remove TST_ERR usage from rtnetlink/netdevice
Date: Thu, 17 Jun 2021 09:40:39 +0100	[thread overview]
Message-ID: <87czsk51fc.fsf@suse.de> (raw)
In-Reply-To: <YMirQoiYRYsDP7Sb@yuki>

Hello,

Cyril Hrubis <chrubis@suse.cz> writes:

> Hi!
>> The test author is guaranteed that the library will not set TST_ERR
>> except via the TEST macro and similar.
>> 
>> Currently the rtnetlink & netdevice API returns 0 on fail and 1 on
>> success. Either TST_ERR or errno is used to store the error
>> number. The commit stays with this scheme except that we only use
>> errno. This means that we have to temporarily save errno when it would
>> be overwritten by a less important operation.
>
> I do not like this fix. If nothing else it's fragile and would make any
> patch review for these libraries much harder.
>
> I would prefer having these functions modified to return the errors
> instead even if I have to do the work.

I think there are some other issues here as well. Like the macros are
not prefixed with SAFE_. A lot of the functions are not used or they are
just used internally, but are not marked as static.

I guess there are more tests on the way that use these
functions. However it is difficult to update the API, if you don't know
how it will be used. I like to have a concrete usecase for what I am
working on, especially if it's not code I have a lot of vision for.

I'd prefer to remove the stuff which is unused before making significant
changes to the library. Also mark internal functions as static and
remove the headers.

Also, it seems the problem functions are actually internal and not used
by the test. If they are marked static, then I don't see any problem
with them being inconsistent with the public API.

This may create more work for Martin, but any sweeping changes made by
myself or Cyril will probably break any WIP tests and need to be
changed.

-- 
Thank you,
Richard.

  reply	other threads:[~2021-06-17  8:40 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-15  7:40 [LTP] [PATCH v4 1/3] Add Coccinelle helper scripts for reference Richard Palethorpe
2021-06-15  7:40 ` [LTP] [PATCH v4 2/3] API: Remove TEST macro usage from library Richard Palethorpe
2021-06-15 13:23   ` Cyril Hrubis
2021-06-16  6:37   ` Li Wang
2021-06-17  7:45     ` Richard Palethorpe
2021-06-15  7:40 ` [LTP] [PATCH v4 3/3] API: Remove TST_ERR usage from rtnetlink/netdevice Richard Palethorpe
2021-06-15 13:29   ` Cyril Hrubis
2021-06-17  8:40     ` Richard Palethorpe [this message]
2021-06-22 13:49       ` Martin Doucha
2021-06-22 13:40         ` Cyril Hrubis
2021-06-22 14:25           ` Martin Doucha
2021-06-22 14:14             ` Cyril Hrubis
2021-06-23 10:24             ` Richard Palethorpe
2021-06-15 13:22 ` [LTP] [PATCH v4 1/3] Add Coccinelle helper scripts for reference Cyril Hrubis
2021-06-16  7:24 ` Li Wang
2021-06-16  7:48   ` Li Wang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87czsk51fc.fsf@suse.de \
    --to=rpalethorpe@suse.de \
    --cc=ltp@lists.linux.it \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox