LKML Archive on lore.kernel.org
 help / color / Atom feed
From: Steven Rostedt <rostedt@goodmis.org>
To: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	Chen Yu <yu.c.chen@intel.com>, Chen Yu <yu.chen.surf@gmail.com>,
	LKML <linux-kernel@vger.kernel.org>,
	Ingo Molnar <mingo@kernel.org>, Jonathan Corbet <corbet@lwn.net>
Subject: Re: [PATCH v4 3/4] tools/bootconfig: Align the bootconfig applied initrd image size to 4
Date: Wed, 18 Nov 2020 11:22:49 -0500
Message-ID: <20201118112249.30d20147@gandalf.local.home> (raw)
In-Reply-To: <160571374406.277955.187006985015101129.stgit@devnote2>

On Thu, 19 Nov 2020 00:35:44 +0900
Masami Hiramatsu <mhiramat@kernel.org> wrote:

> +
> +	/* To align up the total size to BOOTCONFIG_ALIGN, get padding size */
> +	total_size = stat.st_size + size + sizeof(u32) * 2 + BOOTCONFIG_MAGIC_LEN;
> +	pad = BOOTCONFIG_ALIGN - total_size % BOOTCONFIG_ALIGN;
> +	if (pad == BOOTCONFIG_ALIGN)
> +		pad = 0;

If alignment is always a power of two, you could simply do:

	pad = (total_size + BOOTCONFIG_ALIGN - 1) &  ~(BOOTCONFIG_ALIGN - 1)) - total_size;

Which will give you the proper padding, without the if.

> +	size += pad;
> +
> +	/* Add a footer */
> +	*(u32 *)(data + size) = size;
> +	*(u32 *)(data + size + sizeof(u32)) = csum;
> +	memcpy(data + size + sizeof(u32) * 2, BOOTCONFIG_MAGIC, BOOTCONFIG_MAGIC_LEN);
> +	total_size = size + sizeof(u32) * 2 + BOOTCONFIG_MAGIC_LEN;

I wonder if it would be cleaner to just have a void pointer index for the above:

	void *p;

	p = data + size;
	*(u32 *)p = size;
	p += sizeof(u32);

	*(u32 *)p = csum;
	p += sizeof(u32);

	memcpy(p, BOOTCONFIG_MAGIC, BOOTCONFIG_MAGIC_LEN);
	p += BOOTCONFIG_MAGIC_LEN;

	total_size = p - (void *)data;

Also, how does this work if we run this on a little endian box for a big
endian crossbuild?

-- Steve


> +
> +	ret = write(fd, data, total_size);
> +	if (ret < total_size) {
>  		if (ret < 0)
>  			ret = -errno;
>  		pr_err("Failed to apply a boot config: %d\n", ret);
> -		if (ret < 0)
> -			goto out;
> -		goto out_rollback;
> -	}
> -	/* Write a magic word of the bootconfig */
> -	ret = write(fd, BOOTCONFIG_MAGIC, BOOTCONFIG_MAGIC_LEN);
> -	if (ret < BOOTCONFIG_MAGIC_LEN) {
> -		if (ret < 0)
> -			ret = -errno;
> -		pr_err("Failed to apply a boot config magic: %d\n", ret);
> -		goto out_rollback;
> -	}
> -	ret = 0;
> +		if (ret > 0)
> +			goto out_rollback;
> +	} else
> +		ret = 0;
> +
>  out:
>  	close(fd);
>  	free(data);

  reply index

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-18 15:35 [PATCH v4 0/4] tools/bootconfig: Align the bootconfig applied initrd Masami Hiramatsu
2020-11-18 15:35 ` [PATCH v4 1/4] tools/bootconfig: Fix errno reference after printf() Masami Hiramatsu
2020-11-18 15:35 ` [PATCH v4 2/4] tools/bootconfig: Fix to check the write failure correctly Masami Hiramatsu
2020-11-18 16:04   ` Steven Rostedt
2020-11-19  1:41     ` Masami Hiramatsu
2020-11-19 13:59       ` Steven Rostedt
2020-11-19  5:42   ` Masami Hiramatsu
2020-11-18 15:35 ` [PATCH v4 3/4] tools/bootconfig: Align the bootconfig applied initrd image size to 4 Masami Hiramatsu
2020-11-18 16:22   ` Steven Rostedt [this message]
2020-11-19  2:32     ` Masami Hiramatsu
2020-11-18 15:35 ` [PATCH v4 4/4] docs: bootconfig: Update file format on initrd image Masami Hiramatsu

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=20201118112249.30d20147@gandalf.local.home \
    --to=rostedt@goodmis.org \
    --cc=corbet@lwn.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mhiramat@kernel.org \
    --cc=mingo@kernel.org \
    --cc=torvalds@linux-foundation.org \
    --cc=yu.c.chen@intel.com \
    --cc=yu.chen.surf@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

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git
	git clone --mirror https://lore.kernel.org/lkml/8 lkml/git/8.git
	git clone --mirror https://lore.kernel.org/lkml/9 lkml/git/9.git
	git clone --mirror https://lore.kernel.org/lkml/10 lkml/git/10.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org
	public-inbox-index lkml

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git