[PATCH net] net: rnpgbe: Pass an expression directly in rnpgbe_rm_adapter()

[PATCH net] net: rnpgbe: Pass an expression directly in rnpgbe_rm_adapter()

[Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 12 Jul 2026 20:25:10 +0200

The address of a data structure member was determined before
a corresponding null pointer check in the implementation of
the function “rnpgbe_rm_adapter”.

Thus avoid the risk for undefined behaviour by omitting the variable “hw”.
Pass the required address directly to a function call.

This issue was detected by using the Coccinelle software.

Fixes: 2ee95ec17e97c58b65e978a08b75fa8cb6424e4e ("net: rnpgbe: Add register_netdev")
Cc: stable@vger.kernel.org
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
index 70a2b0082ba8..93cf757c951c 100644
--- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
@@ -245,7 +245,6 @@ static int rnpgbe_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 static void rnpgbe_rm_adapter(struct pci_dev *pdev)
 {
 	struct mucse *mucse = pci_get_drvdata(pdev);
-	struct mucse_hw *hw = &mucse->hw;
 	struct net_device *netdev;
 	int err;
 
@@ -253,7 +252,7 @@ static void rnpgbe_rm_adapter(struct pci_dev *pdev)
 		return;
 	netdev = mucse->netdev;
 	unregister_netdev(netdev);
-	err = rnpgbe_send_notify(hw, false, mucse_fw_powerup);
+	err = rnpgbe_send_notify(&mucse->hw, false, mucse_fw_powerup);
 	if (err)
 		dev_warn(&pdev->dev, "Send powerdown to hw failed %d\n", err);
 	free_netdev(netdev);
-- 
2.55.0

Re: [PATCH net] net: rnpgbe: Pass an expression directly in rnpgbe_rm_adapter()

[Vadim Fedorenko]

On 12/07/2026 19:35, Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sun, 12 Jul 2026 20:25:10 +0200
> 
> The address of a data structure member was determined before
> a corresponding null pointer check in the implementation of
> the function “rnpgbe_rm_adapter”.

The null check in rnpgbe_rm_adapter() looks more like defensive
programming, which got into the code unnoticed. pdev will not exist if
netdev is not allocated, where mucse is netdev private data.

Re: net: rnpgbe: Pass an expression directly in rnpgbe_rm_adapter()

[Markus Elfring]

>> The address of a data structure member was determined before
>> a corresponding null pointer check in the implementation of
>> the function “rnpgbe_rm_adapter”.
> 
> The null check in rnpgbe_rm_adapter() looks more like defensive
> programming,

This can be fine, can't it?


>              which got into the code unnoticed.

Some patch reviews happened accordingly.


>                                                 pdev will not exist if
> netdev is not allocated, where mucse is netdev private data.

Would you like to reconsider the relevance of another null pointer check?

Regards,
Markus

Re: [PATCH net] net: rnpgbe: Pass an expression directly in rnpgbe_rm_adapter()

[Dan Carpenter]

On Sun, Jul 12, 2026 at 08:35:21PM +0200, Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sun, 12 Jul 2026 20:25:10 +0200
> 
> The address of a data structure member was determined before
> a corresponding null pointer check in the implementation of
> the function “rnpgbe_rm_adapter”.
> 
> Thus avoid the risk for undefined behaviour by omitting the variable “hw”.
> Pass the required address directly to a function call.
> 
> This issue was detected by using the Coccinelle software.
> 
> Fixes: 2ee95ec17e97c58b65e978a08b75fa8cb6424e4e ("net: rnpgbe: Add register_netdev")

There is no NULL dereference here.  It's just pointer math.
No need for a Fixes tag.

regards,
dan carpenter

Re: [PATCH net] net: rnpgbe: Pass an expression directly in rnpgbe_rm_adapter()

[Markus Elfring]

>> The address of a data structure member was determined before
>> a corresponding null pointer check in the implementation of
>> the function “rnpgbe_rm_adapter”.
>>
>> Thus avoid the risk for undefined behaviour by omitting the variable “hw”.
>> Pass the required address directly to a function call.
>>
>> This issue was detected by using the Coccinelle software.
>>
>> Fixes: 2ee95ec17e97c58b65e978a08b75fa8cb6424e4e ("net: rnpgbe: Add register_netdev")
> 
> There is no NULL dereference here.  It's just pointer math.
> No need for a Fixes tag.

How does your view fit to information in an article like “Fun with NULL pointers, part 1”(by Jonathan Corbet from 2009-07-20)?
https://lwn.net/Articles/342330/

Regards,
Markus

Re: [PATCH net] net: rnpgbe: Pass an expression directly in rnpgbe_rm_adapter()

[Andrew Lunn]

On Sun, Jul 12, 2026 at 08:35:21PM +0200, Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sun, 12 Jul 2026 20:25:10 +0200
> 
> The address of a data structure member was determined before
> a corresponding null pointer check in the implementation of
> the function “rnpgbe_rm_adapter”.

Please can you show the path taken that mucse is actually NULL.

I tend to agree, that the correct change is to remove the NULL pointer
test.

    Andrew

---
pw-bot: cr

Re: net: rnpgbe: Pass an expression directly in rnpgbe_rm_adapter()

[Markus Elfring]

>> The address of a data structure member was determined before
>> a corresponding null pointer check in the implementation of
>> the function “rnpgbe_rm_adapter”.
> 
> Please can you show the path taken that mucse is actually NULL.

The applied sanity check indicates a corresponding possibility.

Which result representations would you find helpful for the requested
data flow analysis?


> I tend to agree, that the correct change is to remove the NULL pointer
> test.
Will the omission of such a check gain wider acceptance?

Regards,
Markus

Re: [PATCH net] net: rnpgbe: Pass an expression directly in rnpgbe_rm_adapter()

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 2791 bytes --]

On Mon, Jul 13, 2026 at 10:22:06AM +0200, Markus Elfring wrote:
> >> The address of a data structure member was determined before
> >> a corresponding null pointer check in the implementation of
> >> the function “rnpgbe_rm_adapter”.
> >>
> >> Thus avoid the risk for undefined behaviour by omitting the variable “hw”.
> >> Pass the required address directly to a function call.
> >>
> >> This issue was detected by using the Coccinelle software.
> >>
> >> Fixes: 2ee95ec17e97c58b65e978a08b75fa8cb6424e4e ("net: rnpgbe: Add register_netdev")
> > 
> > There is no NULL dereference here.  It's just pointer math.
> > No need for a Fixes tag.
> 
> How does your view fit to information in an article like “Fun with
> NULL pointers, part 1”(by Jonathan Corbet from 2009-07-20)?
> https://lwn.net/Articles/342330/

It does fit, because the problematic code discussed in Jonathan Corbet's
article is of the type:

	int i = ptr->i;

	if (!ptr)
		do_something();

while here we have:

	int *i = &ptr->i;

	if (!ptr)
		do_something();

which at least in my test[1] is relevantly different. Note, I didn't
study the C standard if the compiler is free to optimize out
do_something() also in the 2nd case, but at least today gcc doesn't.

Best regards
Uwe

[1] Me knowing about ARM assembly, that's what I checked:

	$ cat test.c
	#include <stdio.h>
	#include <stdlib.h>

	struct mystruct {
		int i;
		char c;
	};

	int funcdirect(struct mystruct *ptr)
	{
		int i = ptr->i;

		if (!ptr)
			return -1;

		printf("i = %d\n", i);
		return 0;
	}

	int funcindirect(struct mystruct *ptr)
	{
		int *i = &ptr->i;

		if (!ptr)
			return -1;

		printf("i = %d\n", *i);
		return 0;
	}
	$ arm-linux-gnueabihf-gcc -O3 -c test.c
	$ objdump -D test.o

	test.o:     file format elf32-littlearm


	Disassembly of section .text:

	00000000 <funcdirect>:
	   0:	b508      	push	{r3, lr}
	   2:	4603      	mov	r3, r0
	   4:	4803      	ldr	r0, [pc, #12]	@ (14 <funcdirect+0x14>)
	   6:	6819      	ldr	r1, [r3, #0]
	   8:	4478      	add	r0, pc
	   a:	f7ff fffe 	bl	0 <printf>
	   e:	2000      	movs	r0, #0
	  10:	bd08      	pop	{r3, pc}
	  12:	bf00      	nop
	  14:	00000008 	andeq	r0, r0, r8

	00000018 <funcindirect>:
	  18:	b138      	cbz	r0, 2a <funcindirect+0x12>
	  1a:	6801      	ldr	r1, [r0, #0]
	  1c:	4804      	ldr	r0, [pc, #16]	@ (30 <funcindirect+0x18>)
	  1e:	b508      	push	{r3, lr}
	  20:	4478      	add	r0, pc
	  22:	f7ff fffe 	bl	0 <printf>
	  26:	2000      	movs	r0, #0
	  28:	bd08      	pop	{r3, pc}
	  2a:	f04f 30ff 	mov.w	r0, #4294967295	@ 0xffffffff
	  2e:	4770      	bx	lr
	  30:	0000000c 	andeq	r0, r0, ip

So in funcdirect the check is not present, while it is in funcindirect.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: net: rnpgbe: Pass an expression directly in rnpgbe_rm_adapter()

[Markus Elfring]

Am 15.07.26 um 15:12 schrieb Uwe Kleine-König:
> On Mon, Jul 13, 2026 at 10:22:06AM +0200, Markus Elfring wrote:
>>>> The address of a data structure member was determined before
>>>> a corresponding null pointer check in the implementation of
>>>> the function “rnpgbe_rm_adapter”.
>>>>
>>>> Thus avoid the risk for undefined behaviour by omitting the variable “hw”.
>>>> Pass the required address directly to a function call.
>>>>
>>>> This issue was detected by using the Coccinelle software.
>>>>
>>>> Fixes: 2ee95ec17e97c58b65e978a08b75fa8cb6424e4e ("net: rnpgbe: Add register_netdev")
>>>
>>> There is no NULL dereference here.  It's just pointer math.
>>> No need for a Fixes tag.
>>
>> How does your view fit to information in an article like “Fun with
>> NULL pointers, part 1”(by Jonathan Corbet from 2009-07-20)?
>> https://lwn.net/Articles/342330/
> 
> It does fit, because the problematic code discussed in Jonathan Corbet's
> article is of the type:
> 
> 	int i = ptr->i;
> 
> 	if (!ptr)
> 		do_something();
> 
> while here we have:
> 
> 	int *i = &ptr->i;
> 
> 	if (!ptr)
> 		do_something();
> 
> which at least in my test[1] is relevantly different.

Jonathan Corbet provided the following information.

“…
But Herbert's patch added a line which dereferences the pointer prior to the check. That, of course, is a bug.
…”


>                                                       Note, I didn't
> study the C standard

I hope that clarification approaches can evolve further according to this information source.


>                      if the compiler is free to optimize out
> do_something() also in the 2nd case, but at least today gcc doesn't.

Can development interests grow also according to another clarification approach?

Does &((struct name *)NULL -> b) cause undefined behaviour in C11?
https://stackoverflow.com/questions/26906621/does-struct-name-null-b-cause-undefined-behaviour-in-c11



Would you prefer to omit a “sanity check” in the discussed function implementation?

Regards,
Markus

Re: [PATCH net] net: rnpgbe: Pass an expression directly in rnpgbe_rm_adapter()

[Dan Carpenter]

On Mon, Jul 13, 2026 at 10:22:06AM +0200, Markus Elfring wrote:
> >> The address of a data structure member was determined before
> >> a corresponding null pointer check in the implementation of
> >> the function “rnpgbe_rm_adapter”.
> >>
> >> Thus avoid the risk for undefined behaviour by omitting the variable “hw”.
> >> Pass the required address directly to a function call.
> >>
> >> This issue was detected by using the Coccinelle software.
> >>
> >> Fixes: 2ee95ec17e97c58b65e978a08b75fa8cb6424e4e ("net: rnpgbe: Add register_netdev")
> > 
> > There is no NULL dereference here.  It's just pointer math.
> > No need for a Fixes tag.
> 
> How does your view fit to information in an article like “Fun with NULL pointers, part 1”(by Jonathan Corbet from 2009-07-20)?
> https://lwn.net/Articles/342330/
> 

Of course you can't have NULL pointer dereferences but this is not a
dereference, it's just pointer math.

regards,
dan carpenter

Re: net: rnpgbe: Pass an expression directly in rnpgbe_rm_adapter()

[Markus Elfring]

>>>> The address of a data structure member was determined before
>>>> a corresponding null pointer check in the implementation of
>>>> the function “rnpgbe_rm_adapter”.
>>>>
>>>> Thus avoid the risk for undefined behaviour by omitting the variable “hw”.
>>>> Pass the required address directly to a function call.
>>>>
>>>> This issue was detected by using the Coccinelle software.
>>>>
>>>> Fixes: 2ee95ec17e97c58b65e978a08b75fa8cb6424e4e ("net: rnpgbe: Add register_netdev")
>>>
>>> There is no NULL dereference here.  It's just pointer math.
>>> No need for a Fixes tag.
>>
>> How does your view fit to information in an article like “Fun with NULL pointers, part 1”
>> (by Jonathan Corbet from 2009-07-20)?>> https://lwn.net/Articles/342330/
>>
> 
> Of course you can't have NULL pointer dereferences but this is not a
> dereference, it's just pointer math.
Does your understanding of programming language details differ from the view
by Jonathan Corbet?

Regards,
Markus

Re: net: rnpgbe: Pass an expression directly in rnpgbe_rm_adapter()

[Dan Carpenter]

On Wed, Jul 15, 2026 at 08:45:48PM +0200, Markus Elfring wrote:
> 
> Jonathan Corbet provided the following information.
> 
> “…
> But Herbert's patch added a line which dereferences the pointer prior to the check. That, of course, is a bug.
> …”
> 

Notice that it says "dereferences" not "does pointer math".

> 
> >                                                       Note, I didn't
> > study the C standard
> 
> I hope that clarification approaches can evolve further according to this information source.
> 
> 
> >                      if the compiler is free to optimize out
> > do_something() also in the 2nd case, but at least today gcc doesn't.
> 
> Can development interests grow also according to another clarification approach?
> 
> Does &((struct name *)NULL -> b) cause undefined behaviour in C11?
> https://stackoverflow.com/questions/26906621/does-struct-name-null-b-cause-undefined-behaviour-in-c11
> 
> 
> 
> Would you prefer to omit a “sanity check” in the discussed function implementation?
> 

This is irrelevant.

In C writing if (!p) and if (p == NULL) are always equivalent but weirdly
the NULL doesn't have to zero.  It's part of the C FAQ.
https://c-faq.com/null/machexamp.html It's just a bit of fun trivia that
doesn't really matter unless you have a time machine.  Even if you invented
a time machine, the code here would still be fine because we have a NULL
test before dereferencing the results of our pointer math.

regards,
dan carpenter

Re: net: rnpgbe: Pass an expression directly in rnpgbe_rm_adapter()

[Markus Elfring]

> In C writing if (!p) and if (p == NULL) are always equivalent but weirdly
> the NULL doesn't have to zero.  It's part of the C FAQ.
> https://c-faq.com/null/machexamp.html It's just a bit of fun trivia that
> doesn't really matter unless you have a time machine.  Even if you invented
> a time machine, the code here would still be fine because we have a NULL
> test before dereferencing the results of our pointer math.
Which expression evaluations would you interpret as pointer dereferences finally?
https://c-faq.com/sx1/

Regards,
Markus

Re: net: rnpgbe: Pass an expression directly in rnpgbe_rm_adapter()

[Andrew Lunn]

On Thu, Jul 16, 2026 at 11:48:43AM +0200, Markus Elfring wrote:
> > In C writing if (!p) and if (p == NULL) are always equivalent but weirdly
> > the NULL doesn't have to zero.  It's part of the C FAQ.
> > https://c-faq.com/null/machexamp.html It's just a bit of fun trivia that
> > doesn't really matter unless you have a time machine.  Even if you invented
> > a time machine, the code here would still be fine because we have a NULL
> > test before dereferencing the results of our pointer math.
> Which expression evaluations would you interpret as pointer dereferences finally?
> https://c-faq.com/sx1/

This is all interesting, but nobody has yet explain how this function
can be called such that we have a NULL pointer.

    Andrew

Re: net: rnpgbe: Pass an expression directly in rnpgbe_rm_adapter()

[Dan Carpenter]

On Thu, Jul 16, 2026 at 10:20:42AM +0200, Markus Elfring wrote:
> >>>> The address of a data structure member was determined before
> >>>> a corresponding null pointer check in the implementation of
> >>>> the function “rnpgbe_rm_adapter”.
> >>>>
> >>>> Thus avoid the risk for undefined behaviour by omitting the variable “hw”.
> >>>> Pass the required address directly to a function call.
> >>>>
> >>>> This issue was detected by using the Coccinelle software.
> >>>>
> >>>> Fixes: 2ee95ec17e97c58b65e978a08b75fa8cb6424e4e ("net: rnpgbe: Add register_netdev")
> >>>
> >>> There is no NULL dereference here.  It's just pointer math.
> >>> No need for a Fixes tag.
> >>
> >> How does your view fit to information in an article like “Fun with NULL pointers, part 1”
> >> (by Jonathan Corbet from 2009-07-20)?>> https://lwn.net/Articles/342330/
> >>
> > 
> > Of course you can't have NULL pointer dereferences but this is not a
> > dereference, it's just pointer math.
> Does your understanding of programming language details differ from the view
> by Jonathan Corbet?

Jonathan Corbet's article talks about dereferences.

	p = tun->sk;
            ^^^^^^^
This is a dereference.

	p = &tun->sk;
            ^
This is pointer math.  It's not a dereference.

regards,
dan carpenter

Re: net: rnpgbe: Pass an expression directly in rnpgbe_rm_adapter()

[Dan Carpenter]

On Thu, Jul 16, 2026 at 03:24:22PM +0200, Andrew Lunn wrote:
> On Thu, Jul 16, 2026 at 11:48:43AM +0200, Markus Elfring wrote:
> > > In C writing if (!p) and if (p == NULL) are always equivalent but weirdly
> > > the NULL doesn't have to zero.  It's part of the C FAQ.
> > > https://c-faq.com/null/machexamp.html It's just a bit of fun trivia that
> > > doesn't really matter unless you have a time machine.  Even if you invented
> > > a time machine, the code here would still be fine because we have a NULL
> > > test before dereferencing the results of our pointer math.
> > Which expression evaluations would you interpret as pointer dereferences finally?
> > https://c-faq.com/sx1/
> 
> This is all interesting, but nobody has yet explain how this function
> can be called such that we have a NULL pointer.

mucse cannot be NULL.

We call pci_set_drvdata() during probe() and it is still valid when
we call the remove() function.

regards,
dan carpenter

Re: net: rnpgbe: Pass an expression directly in rnpgbe_rm_adapter()

[Andrew Lunn]

On Thu, Jul 16, 2026 at 04:34:05PM +0300, Dan Carpenter wrote:
> On Thu, Jul 16, 2026 at 03:24:22PM +0200, Andrew Lunn wrote:
> > On Thu, Jul 16, 2026 at 11:48:43AM +0200, Markus Elfring wrote:
> > > > In C writing if (!p) and if (p == NULL) are always equivalent but weirdly
> > > > the NULL doesn't have to zero.  It's part of the C FAQ.
> > > > https://c-faq.com/null/machexamp.html It's just a bit of fun trivia that
> > > > doesn't really matter unless you have a time machine.  Even if you invented
> > > > a time machine, the code here would still be fine because we have a NULL
> > > > test before dereferencing the results of our pointer math.
> > > Which expression evaluations would you interpret as pointer dereferences finally?
> > > https://c-faq.com/sx1/
> > 
> > This is all interesting, but nobody has yet explain how this function
> > can be called such that we have a NULL pointer.
> 
> mucse cannot be NULL.
> 
> We call pci_set_drvdata() during probe() and it is still valid when
> we call the remove() function.

Same as what i thought. So while this is an interesting discussion,
please could someone post the real fix, remove the NULL pointer check.

       Andrew

Re: net: rnpgbe: Pass an expression directly in rnpgbe_rm_adapter()

[Jonathan Corbet]

Markus Elfring <Markus.Elfring@web.de> writes:

> Does your understanding of programming language details differ from the view
> by Jonathan Corbet?

I'm not sure why you keep dragging me into this - I have not opined on
this particular situation, which is definitely *not* a dereference.

jon

Re: net: rnpgbe: Pass an expression directly in rnpgbe_rm_adapter()

[Markus Elfring]

> Jonathan Corbet's article talks about dereferences.
> 
> 	p = tun->sk;
>             ^^^^^^^
> This is a dereference.
> 
> 	p = &tun->sk;
>             ^
> This is pointer math.  It's not a dereference.
Can such a development view be confusing?

Is the operator “address of” applied only after a pointer dereference attempt in this case?
https://en.cppreference.com/c/language/operator_member_access

Regards,
Markus

Re: net: rnpgbe: Pass an expression directly in rnpgbe_rm_adapter()

[Markus Elfring]

>> Does your understanding of programming language details differ from the view
>> by Jonathan Corbet?
> 
> I'm not sure why you keep dragging me into this

You are one of the contributors who pointed a specific control flow out as a bug.


>                                                 - I have not opined on
> this particular situation,

I got an other impression.


>                            which is definitely *not* a dereference.
Does such a feedback indicate a questionable communication difficulty?

Regards,
Markus

[PATCH v2 net-next] net: rnpgbe: Delete a null pointer check in rnpgbe_rm_adapter()

[Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 16 Jul 2026 19:15:06 +0200

The check for the pointer “mucse” was finally identified as undesirable.
Thus remove it.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---

v2:
Andrew Lunn requested the removal of a null pointer check.


 drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
index 70a2b0082ba8..82a3fbfb3c0a 100644
--- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
@@ -249,8 +249,6 @@ static void rnpgbe_rm_adapter(struct pci_dev *pdev)
 	struct net_device *netdev;
 	int err;
 
-	if (!mucse)
-		return;
 	netdev = mucse->netdev;
 	unregister_netdev(netdev);
 	err = rnpgbe_send_notify(hw, false, mucse_fw_powerup);
-- 
2.55.0

Re: [PATCH v2 net-next] net: rnpgbe: Delete a null pointer check in rnpgbe_rm_adapter()

[Andrew Lunn]

On Thu, Jul 16, 2026 at 07:26:21PM +0200, Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Thu, 16 Jul 2026 19:15:06 +0200
> 
> The check for the pointer “mucse” was finally identified as undesirable.

I'm not sure finally is appropriate. It was one of the first questions
i asked. Also, i expect anybody looking at a Coccinelle report to take
the time to understand the code and decide what the correct fix is. We
have too many developers doing the minimum to make the tool happy,
without actually thinking.

Please also include the reasoning behind this. The commit message is
all about "Why?" Why is it undesirable?

And lastly, please always start a new thread for a new version of the
patch.

    Andrew

---
pw-bot: cr