All of lore.kernel.org
 help / color / mirror / Atom feed
From: Pete Batard <pete@akeo.ie>
To: grub-devel@gnu.org
Subject: Re: [PATCH] fs: remove implicit compiler calls to memset/memcpy
Date: Mon, 18 Apr 2016 12:13:30 +0200	[thread overview]
Message-ID: <5714B34A.6050501@akeo.ie> (raw)
In-Reply-To: <CAEaD8JO2Yp3RTfNjjqReiHoi=eWxMzWG7qg9=7yBa+Ts0RtTJQ@mail.gmail.com>

On 2016.04.18 07:50, Vladimir 'phcoder' Serbinenko wrote:
> You can use asm to get around msvc limitations. Sth like
>
> .global memcpy
> memcpy:
>        jmp grub_memcpy

Yes I'm well aware I could try to create my own library (or equivalent) 
that redefines memcpy/memset, using some workaround to avoid the MSVC 
compiler error (if needed, I could create a compatible aliasing lib with 
gcc). But that's something I'd prefer to avoid because it's never that 
simple in the MSVC world, and I'm also compiling for multiple archs (x86 
and ARM, possibly more in the future), which makes assembly workarounds 
an annoyance. Besides, I already have to patch the GRUB source anyway, 
with some MSVC specific quirks (some of which I doubt you guys would 
like [1]), so adding some more to my current patch series to remove the 
implicits is not exactly a big deal. But of course, I sure wouldn't mind 
minimizing the amount of code I need to patch to use GRUB in my scope, 
which is the whole point of this submission.

> Where implicit memcpy is inserted is pretty much unpredictable

You're missing the purpose of my request. I'm not asking the GRUB 
project to do anything about predicting where memset/memcpy are going 
inserted, or even attempt to be preemptive about that in the future. I'm 
simply asking it to address the ones that were isolated with 100% 
accuracy (compiler flag to generate assembly with source + grep on said 
assembly) from using the current GRUB source in the specific context 
that I have.
There's no "predictive" component in what I am requesting here, not even 
an implied one.

So, in case that is the crux of the issue, should this patch be 
integrated, I am not asking any GRUB contributor to try to keep 
conscious of or try to identify potential implicits as they go about 
modifying fs or any other parts of the code. It is in fact of little 
consequence if someone comes in and breaks the applied implicits removal 
the day after it is applied, as I am using GRUB as a git submodule, 
therefore it'll remain tied to the specific git rev where it isn't 
broken. And if I try to update the submodule, and identify breakage, I 
will of course submit a new patch to this list as needed.

> and we're
> not going to maintain memcpy-free environment because of this

Again, this is not what I am asking. This is a simple "you scratch my 
back, I'll scratch yours" request, that, in a way, is pretty much akin 
to asking the GRUB project to add extra parenthesis in _very specific_ 
parts of source, to make it more palatable when these specific parts are 
used with compilers that are not officially supported by the project, 
and as a one-off thing (i.e. without asking for anyone to be tasked to 
maintaining that new parenthesis layout going forward).

If you want to say: "Well, we don't official support MSVC, so we're not 
going to pick a patch that is essentially aimed at improving MSVC 
support, especially if only applies to a limited set of sources", that's 
fine with me.
But please don't try to imply that the patch has a much larger scope 
than it actually does, as your justification for rejection, or that it 
is going to require any extra work/maintainance from GRUB contributors 
once applied.

Regards,

/Pete

[1] 
https://github.com/pbatard/efifs/blob/master/0001-GRUB-fixes-for-MSVC-compilation-part-1.patch#L366-L410


  reply	other threads:[~2016-04-18 10:13 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-04-10 13:52 [PATCH] fs: remove implicit compiler calls to memset/memcpy Pete Batard
2016-04-10 14:30 ` Vladimir 'phcoder' Serbinenko
2016-04-10 15:50   ` Pete Batard
2016-04-18  5:50     ` Vladimir 'phcoder' Serbinenko
2016-04-18 10:13       ` Pete Batard [this message]
2016-04-18 10:36         ` Andrei Borzenkov
2016-04-18 10:49           ` Vladimir 'phcoder' Serbinenko
2016-04-18 12:03             ` Pete Batard

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=5714B34A.6050501@akeo.ie \
    --to=pete@akeo.ie \
    --cc=grub-devel@gnu.org \
    /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.