public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: AKASHI Takahiro <takahiro.akashi@linaro.org>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH v2 04/23] fs: fat: make directory iterator global for write use
Date: Thu, 6 Sep 2018 11:29:31 +0900	[thread overview]
Message-ID: <20180906022930.GG18483@linaro.org> (raw)
In-Reply-To: <cd6d82d8-1610-703b-c85e-10d8c050e2f5@suse.de>

On Wed, Sep 05, 2018 at 10:16:32AM +0200, Alexander Graf wrote:
> 
> 
> On 05.09.18 04:14, AKASHI Takahiro wrote:
> > On Tue, Sep 04, 2018 at 12:57:54PM +0200, Alexander Graf wrote:
> >>
> >>
> >>> Am 04.09.2018 um 12:50 schrieb Heinrich Schuchardt <xypron.glpk@gmx.de>:
> >>>
> >>>
> >>>
> >>>> On 09/04/2018 11:01 AM, Alexander Graf wrote:
> >>>>
> >>>>
> >>>>> On 04.09.18 09:49, AKASHI Takahiro wrote:
> >>>>> Directory iterator was introduced in major re-work of read operation by
> >>>>> Rob. We want to use it for write operation extensively as well.
> >>>>
> >>>> Please indicate in the commit message that write operations are
> >>>> implemented in a different .c file and so we have to export the
> >>>> respective functions.
> >>>
> >>> Why? Look at this ugly code:
> >>>
> >>> fs/fat/fat_write.c:17:#include "fat.c"
> >>
> >> In that case we don't need this patch at all, no?
> > 
> > Oops, I didn't notice this before.
> > If, however, "include fat.c" makes any sense, theoretically we don't need
> > "depends on FS_FAT" for FS_FAT_WRITE.
> > There seems to be a contradiction between the code and config.
> 
> I'm not sure it's contradictory. Someone just implemented a poor man's
> LTO by including the 2 files with each other.

Well, LTO has long existed since gcc4.8 ...
(and improvement would be quite small, I guess.)

> > I prefer just to remove the line, '#include "fat.c"' from fat_write.c
> > and add more "extern" definitions in fat.h if necessary.
> 
> We can worry about refactoring things later down the road. By making
> functions go external, we lose the compiler's ability to inline
> functions which may again bloat code which may trip random boards above
> size limits.
> 
> For now, I'd suggest you leave the ugly #include "fat.c" in and not move
> anything into fat.h. That way the file is self-contained and you don't
> have to worry about exporting just yet.

Although I won't tolerate such an ugly code, I will follow your suggestion
mainly because lots of existing defconfigs enable FAT_WRITE without
FS_FAT.

Thanks,
-Takahiro AKASHI

> 
> Alex

  reply	other threads:[~2018-09-06  2:29 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-04  7:49 [U-Boot] [PATCH v2 00/23] subject: fs: fat: extend FAT write operations AKASHI Takahiro
2018-09-04  7:49 ` [U-Boot] [PATCH v2 01/23] fs: fat: guard the content of include/fat.h AKASHI Takahiro
2018-09-04  8:52   ` Alexander Graf
2018-09-04 10:46     ` Heinrich Schuchardt
2018-09-05  1:54       ` AKASHI Takahiro
2018-09-05  5:53         ` Heinrich Schuchardt
2018-09-05  1:14     ` AKASHI Takahiro
2018-09-04  7:49 ` [U-Boot] [PATCH v2 02/23] fs: fat: extend get_fs_info() for write use AKASHI Takahiro
2018-09-04  7:49 ` [U-Boot] [PATCH v2 03/23] fs: fat: handle "." and ".." of root dir correctly with fat_itr_resolve() AKASHI Takahiro
2018-09-04  7:49 ` [U-Boot] [PATCH v2 04/23] fs: fat: make directory iterator global for write use AKASHI Takahiro
2018-09-04  9:01   ` Alexander Graf
2018-09-04 10:50     ` Heinrich Schuchardt
2018-09-04 10:57       ` Alexander Graf
2018-09-05  2:14         ` AKASHI Takahiro
2018-09-05  8:16           ` Alexander Graf
2018-09-06  2:29             ` AKASHI Takahiro [this message]
2018-09-04  7:49 ` [U-Boot] [PATCH v2 05/23] fs: fat: assure iterator's ->dent belongs to ->clust AKASHI Takahiro
2018-09-04  7:49 ` [U-Boot] [PATCH v2 06/23] Revert "fs: fat: cannot write to subdirectories" AKASHI Takahiro
2018-09-04  7:49 ` [U-Boot] [PATCH v2 07/23] fs: fat: check and normailze file name AKASHI Takahiro
2018-09-04  7:49 ` [U-Boot] [PATCH v2 08/23] fs: fat: write returns error code instead of -1 AKASHI Takahiro
2018-09-04  7:49 ` [U-Boot] [PATCH v2 09/23] fs: fat: support write with sub-directory path AKASHI Takahiro
2018-09-04  7:49 ` [U-Boot] [PATCH v2 10/23] fs: fat: refactor write interface for a file offset AKASHI Takahiro
2018-09-04  7:49 ` [U-Boot] [PATCH v2 11/23] fs: fat: support write with non-zero offset AKASHI Takahiro
2018-09-04  7:49 ` [U-Boot] [PATCH v2 12/23] cmd: fat: add offset parameter to fatwrite AKASHI Takahiro
2018-09-04  7:49 ` [U-Boot] [PATCH v2 13/23] fs: add mkdir interface AKASHI Takahiro
2018-09-04  7:49 ` [U-Boot] [PATCH v2 14/23] fs: fat: remember the starting cluster number of directory AKASHI Takahiro
2018-09-04  7:49 ` [U-Boot] [PATCH v2 15/23] fs: fat: support mkdir AKASHI Takahiro
2018-09-04  7:49 ` [U-Boot] [PATCH v2 16/23] cmd: fat: add fatmkdir command AKASHI Takahiro
2018-09-04  7:49 ` [U-Boot] [PATCH v2 17/23] efi_loader: file: support creating a directory AKASHI Takahiro
2018-09-04  7:49 ` [U-Boot] [PATCH v2 18/23] efi_loader: implement a pseudo "file delete" AKASHI Takahiro
2018-09-04  9:16   ` Alexander Graf
2018-09-05  2:51     ` AKASHI Takahiro
2018-09-05  8:22       ` Alexander Graf
2018-09-06  2:43         ` AKASHI Takahiro
2018-09-06  6:09           ` Alexander Graf
2018-09-04  7:49 ` [U-Boot] [PATCH v2 19/23] fs-test: fix false positive error at Test Case 12 AKASHI Takahiro
2018-09-04  7:49 ` [U-Boot] [PATCH v2 20/23] fs-test: update the test result as of v2018.09 AKASHI Takahiro
2018-09-04  7:49 ` [U-Boot] [PATCH v2 21/23] test/py: convert fs-test.sh to pytest AKASHI Takahiro
2018-09-14 10:54   ` Simon Glass
2018-09-25  4:47     ` AKASHI Takahiro
2018-09-04  7:49 ` [U-Boot] [PATCH v2 22/23] test/py: fs: add extended write operation test AKASHI Takahiro
2018-09-04  7:49 ` [U-Boot] [PATCH v2 23/23] test/py: fs: add fstest/mkdir test AKASHI Takahiro

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=20180906022930.GG18483@linaro.org \
    --to=takahiro.akashi@linaro.org \
    --cc=u-boot@lists.denx.de \
    /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