[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
signature.asc
Description: PGP signature
[Qemu-devel] [PATCH 1/3] nvram: at24c: remove doubled prefix for ERR, Wolfram Sang, 2018/03/12
[Qemu-devel] [PATCH 2/3] nvram: at24c: prevent segfault by checking "rom-size", Wolfram Sang, 2018/03/12