All of lore.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.