On Thursday 20 September 2007 03:07:00 pm Christoph Bartelmus wrote:
> Jarod Wilson "jarod@..." wrote:
> > On Sunday 09 September 2007 04:45:00 pm Christoph Bartelmus wrote:
> >> I think the code that uses alloc_status, mem_faiure, etc. should be =A0
> >> rewritten to use goto or similar. At least the changes to avoid =A0
> >> assignments in if conditions were not vaild, so I had to undo some of =
> >> them.
> > Does the attached patch snippet make sense as a route to go? If so, I'll
> > apply the same treatment to other remaining similar sections.
> Sorry for the late reply.
> I think we should get rid of the alloc_status variable altogether.
> Instead you can add some labels with proper names and use a goto with
> the according label, e.g. goto kmalloc_imon_context_failed;.
I presume that means you'd also like to drop the mem_failure variable in=20
things like lirc_igorplugusb as well? Certainly can be done, but we'll lose=
bit of debugging info from the printk before returning -ENOMEM (at least in=
the igor code).
Speaking of the igor code... Upon looking at the mem_failure switch in=20
lirc_igorplugusb.c in greater detail, it flat-out looks wrong. You hit the=
switch, then only free a tiny portion of memory allocated, and only=20
return -ENOMEM if the mem_failure was case 1. Otherwise, we continue on our=
Here's a patched up implementation of the igor code that still uses the=20
mem_failure variable to track where we failed, but does so without a switch=
and I believe correctly cleans up memory:
If you do indeed want to drop mem_failure altogether, simple enough to rip =
out of that version. Either way, let me know your thoughts, and I can give=
the same treatment to the other drivers with similar code.