Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1384813

RE: [PATCH 1/7] debugobjects: make fixup functions return bool instead of int

Show all headers | View raw

On Fri, 22 Apr 2016, Du, Changbin wrote:
> > On Fri, 22 Apr 2016, changbin.du@intel.com wrote:
> > > A bad thing is that debug_object_fixup use the return value for
> > > arithmetic operation. It confused me that what is the reall return
> > 
> > What's bad about that? The fact that it's used for arithmethic operation or
> > that it confused you?
> > 
> It confused me because this is not a common usage. I was confused that what
> does he fixup function return? A countable value? But doc says return fixed
> or not!

It says return 0 for not fixed up and 1 for fixed up. The activate fixup is
special and it has been written this way to handle the static initialization
case.

> 	if (fixup)
> 		fixed = fixup(addr, state);
> 	debug_objects_fixups += fixed;

> In common,for int return 0 indicates success, negative for fail, positive
> for something countable. So I think it is better follow this rule. Here is
> not of countable, it is Boolean.

Yes, it's common for most of the code. This code has been deliberately been
written differently. I'm not opposed to change that and improve it, but just
slapping bool on it does not really make any difference.

> So why not this?
> 	if (fixup && fixup(addr, state))
> 		debug_objects_fixups++;

There is no problem with that per se.
 
> > > +	bool (*fixup_init)(void *addr, enum debug_obj_state state);
> > > +	bool (*fixup_activate)(void *addr, enum debug_obj_state state);
> > > +	bool (*fixup_destroy)(void *addr, enum debug_obj_state state);
> > > +	bool (*fixup_free)(void *addr, enum debug_obj_state state);
> > > +	bool (*fixup_assert_init)(void *addr, enum debug_obj_state state);
> > 
> > So this change will introduce a gazillion of compile warnings because the
> > callbacks in the various usage sites are still having 'int' return type.
> > 
> No, I modified all the code who use debugojects API.

You do that in the later patches. But patches must be compilable and
functional on their own. Compiling this one will emit a gazillion of
"initialization from incompatible pointer type" warnings.

Thanks,

	tglx

Back to linux.kernel | Previous | Next — Previous in thread | Find similar | Unroll thread

Thread

[PATCH 1/7] debugobjects: make fixup functions return bool instead of int changbin.du@intel.com - 2016-04-22 10:20 +0200
  Re: [PATCH 1/7] debugobjects: make fixup functions return bool  instead of int Thomas Gleixner <tglx@linutronix.de> - 2016-04-22 10:40 +0200
    RE: [PATCH 1/7] debugobjects: make fixup functions return bool  instead of int "Du, Changbin" <changbin.du@intel.com> - 2016-04-22 11:00 +0200
      RE: [PATCH 1/7] debugobjects: make fixup functions return bool  instead of int Thomas Gleixner <tglx@linutronix.de> - 2016-04-22 11:20 +0200

csiph-web