Re: CVS commit: [thorpej-i2c-spi-conf] src/sys/dev/i2c

2021-05-16 Thread Jason Thorpe


> On May 16, 2021, at 1:06 PM, matthew green  wrote:
> 
>> Modified Files:
>>  src/sys/dev/i2c [thorpej-i2c-spi-conf]: spdmem_i2c.c
>> 
>> Log Message:
>> The last change had an unfortunate side-effect on empty DIMM slots, so
>> roll that back.  Instead, if we used direct config, then probe for the
>> module in the attach routine and report if the module is not present,
>> rather than assuming that it is.
> 
> this seems odd to me.
> 
> if we're using direct config, shouldn't match() be fully
> able to determine present or not, avoiding having to have
> an attach() that fake-fails?

The problem is if a device tree has a node for a SPD DIMM, but the DIMM *isn’t 
actually present* (i.e. the node is for “DIMM might live here, go check!”).  
Returning “no match” in this situation would result in a “… not configured” 
message for a non-existent DIMM.

> 
> thanks.
> 
> 
> .mrg.

-- thorpej



re: CVS commit: [thorpej-i2c-spi-conf] src/sys/dev/i2c

2021-05-16 Thread matthew green
> Modified Files:
>   src/sys/dev/i2c [thorpej-i2c-spi-conf]: spdmem_i2c.c
>
> Log Message:
> The last change had an unfortunate side-effect on empty DIMM slots, so
> roll that back.  Instead, if we used direct config, then probe for the
> module in the attach routine and report if the module is not present,
> rather than assuming that it is.

this seems odd to me.

if we're using direct config, shouldn't match() be fully
able to determine present or not, avoiding having to have
an attach() that fake-fails?

thanks.


.mrg.


Re: CVS commit: xsrc/external/mit/xterm/dist

2021-05-16 Thread Roland Illig


16.05.2021 10:29:48 Robert Elz :
> Actually, it isn't.  Consistency is good,
> but correctness is better.  Both is better
> still.  It is possible that unsigned long
> is 32 bits, and size_t is 64, in which case
> casting to unsigned long risks truncating
> the value.  That might not be possible in
> the cases in question, but why risk it?
> Always use the z modifier to print size_t.

My second reason for doing the cast instead of the simpler %zu was that the 
xterm code looks compatible to C90, and I didn't want to destroy this property 
by using %zu, which has only been added in C99.


Re: CVS commit: xsrc/external/mit/xterm/dist

2021-05-16 Thread Robert Elz
Date:Sun, 16 May 2021 16:32:55 +1000
From:Simon Burge 
Message-ID:  <20210516063255.13f704e...@thoreau.thistledown.com.au>

  | Roland Illig wrote:
  | > therefore I did it this way, for consistency.
  |
  | Good reason :)

Actually, it isn't.  Consistency is good,
but correctness is better.  Both is better
still.  It is possible that unsigned long
is 32 bits, and size_t is 64, in which case
casting to unsigned long risks truncating
the value.  That might not be possible in
the cases in question, but why risk it?
Always use the z modifier to print size_t.

kre


Re: CVS commit: xsrc/external/mit/xterm/dist

2021-05-16 Thread Simon Burge
Roland Illig wrote:

> 
> 16.05.2021 04:30:06 Simon Burge :
>
> > Hi Roland,
> >
> > Would using "%zu" and reverting the cast be a better fix here?
>
> I had considered this as well but found another instance in the same
> file where a size_t was fed to printf by casting it to unsigned long,
> therefore I did it this way, for consistency.

Good reason :)

Cheers,
Simon.