qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 3/3] nvram: at24c: use a sane default for "rom-s


From: Wolfram Sang
Subject: Re: [Qemu-devel] [PATCH 3/3] nvram: at24c: use a sane default for "rom-size"
Date: Mon, 19 Mar 2018 09:33:55 +0100
User-agent: NeoMutt/20170113 (1.7.2)

Hi Philippe,

> > I don't mind much, but why? My reasoning was "let's first fix the cause
> > and then the symptom"?
> 
> The '0' case is worst than incorrect, it segfaults, so you are right :)

Ok, thanks.

> >> Can you add a #define for this value? Such AT24C_ROMSIZE_MIN.
> > 
> > Can do, of course. But won't that give room for regressions because
> > people are already using it with lower values?
> 
> Your patch already introduce the regression :)

Not really. The current default setting (0) segfaults. This is how I
discovered it. So, there can't be any active user of that.

> I prefer self-explanatory #defines than magic value, but I see your
> point, so if we can not decide a value, can you add a comment to explain
> the magic value? I think the clearer is to add a #define with a comment.

I wouldn't mind a macro AT24C_ROMSIZE_DEFAULT and a comment explaining
why we chose this value. This is totally fine with me.

It was just the MIN value I saw potential usage regressions with.

> IMO there are too many AT24C eeproms to model, so the "rom-size"
> variable is the easiest way. Your patch #2 is simple enough to avoid the
> #DIV/0!

Fine with me, too. I will update my series accordingly.

Thanks,

   Wolfram

Attachment: signature.asc
Description: PGP signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]