public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Greg KH <gregkh@linuxfoundation.org>
To: Wang YanQing <udknight@gmail.com>,
	jslaby@suse.cz, airlied@redhat.com, akpm@linux-foundation.org,
	kilobyte@angband.pl, peter@hurleysoftware.com,
	rosslagerwall@gmail.com, tiwai@suse.de,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v4]vt: use vc_allocate in con_init
Date: Wed, 8 Jan 2014 20:36:21 -0800	[thread overview]
Message-ID: <20140109043621.GC28887@kroah.com> (raw)
In-Reply-To: <20140107231310.GA3397@udknight>

On Wed, Jan 08, 2014 at 07:13:10AM +0800, Wang YanQing wrote:
> After a5f4f52e82114e85aa1a066bd1a450acc19a464d
> ("vt: use kzalloc() instead of the bootmem allocator"),
> con_init began to use kzalloc to initialize vc_data,
> this patch convert con_init to use vc_allocate.
> 
> The benefit we get:
> 1: reduce code duplication
> 2: vc_allocate is more robust
> 3: use kmalloc instead of kzalloc for vc_screenbuf
> 
> Signed-off-by: Wang YanQing <udknight@gmail.com>
> ---
>  this patch don't have v2 :) I jump to v3 directly
>  from v1, but I keep the mistake for less confusion.
> 
>  Changes v3-v4:
>  1: use bool/true/false instead of int/1/0
> 
>  Thanks for Jiri Slaby
> 
>  drivers/tty/vt/vt.c       | 28 ++++++++++++++--------------
>  drivers/tty/vt/vt_ioctl.c |  8 ++++----
>  include/linux/vt_kern.h   |  2 +-
>  3 files changed, 19 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
> index 15aaa01..c0d6255 100644
> --- a/drivers/tty/vt/vt.c
> +++ b/drivers/tty/vt/vt.c
> @@ -748,7 +748,8 @@ static void visual_init(struct vc_data *vc, int num, int init)
>  	vc->vc_screenbuf_size = vc->vc_rows * vc->vc_size_row;
>  }
>  
> -int vc_allocate(unsigned int currcons)	/* return 0 on success */
> +/* return 0 on success */
> +int vc_allocate(unsigned int currcons, bool early)

What does "early" mean?  I understand the goal of making the code
smaller (hint, it's the same size overall with this change), but adding
flags like this just makes things a whole lot harder to debug and
understand later on.

I really don't like this as it adds to the programmer's complexity at
the tradeoff of a chance at smaller code.

sorry,

greg k-h

      reply	other threads:[~2014-01-09  4:36 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-01-07 23:13 [PATCH v4]vt: use vc_allocate in con_init Wang YanQing
2014-01-09  4:36 ` Greg KH [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140109043621.GC28887@kroah.com \
    --to=gregkh@linuxfoundation.org \
    --cc=airlied@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=jslaby@suse.cz \
    --cc=kilobyte@angband.pl \
    --cc=linux-kernel@vger.kernel.org \
    --cc=peter@hurleysoftware.com \
    --cc=rosslagerwall@gmail.com \
    --cc=tiwai@suse.de \
    --cc=udknight@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox