* [PATCH 1/2] MIPS: host_detect_local_cpu, init ret with concat [PR112759]
@ 2023-12-18 3:44 YunQiang Su
2023-12-18 3:44 ` [PATCH 2/2] libiberty/reconcat: Add note about append string to NULL YunQiang Su
2023-12-18 8:10 ` [PATCH 1/2] MIPS: host_detect_local_cpu, init ret with concat [PR112759] Jakub Jelinek
0 siblings, 2 replies; 5+ messages in thread
From: YunQiang Su @ 2023-12-18 3:44 UTC (permalink / raw)
To: gcc-patches
Cc: ian, pinskia, matoro_mailinglist_gcc-patches, jakub, YunQiang Su
The function `reconcat` cannot append string(s) to NULL,
as the concat process will stop at the first NULL.
Let's initialize `ret` with `concat (" ", NULL)`, then
it can be used by reconcat.
gcc/
PR target/112759
* config/mips/driver-native.cc (host_detect_local_cpu):
initialize ret with concat, so that it can be used by
reconcat later.
---
gcc/config/mips/driver-native.cc | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/gcc/config/mips/driver-native.cc b/gcc/config/mips/driver-native.cc
index afc276f5278..471d1925eff 100644
--- a/gcc/config/mips/driver-native.cc
+++ b/gcc/config/mips/driver-native.cc
@@ -44,7 +44,7 @@ const char *
host_detect_local_cpu (int argc, const char **argv)
{
const char *cpu = NULL;
- char *ret = NULL;
+ char *ret = concat(" ", NULL);
char buf[128];
FILE *f;
bool arch;
--
2.39.2
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 2/2] libiberty/reconcat: Add note about append string to NULL
2023-12-18 3:44 [PATCH 1/2] MIPS: host_detect_local_cpu, init ret with concat [PR112759] YunQiang Su
@ 2023-12-18 3:44 ` YunQiang Su
2023-12-18 7:56 ` Jakub Jelinek
2023-12-18 8:10 ` [PATCH 1/2] MIPS: host_detect_local_cpu, init ret with concat [PR112759] Jakub Jelinek
1 sibling, 1 reply; 5+ messages in thread
From: YunQiang Su @ 2023-12-18 3:44 UTC (permalink / raw)
To: gcc-patches
Cc: ian, pinskia, matoro_mailinglist_gcc-patches, jakub, YunQiang Su
For reconcat, if the `optr` can only be used as the last one
of string list, aka, we cannot append something to it.
Let's add some note into the document.
libiberty:
* concat.c (reconcat): Add note about append string to NULL
into document.
---
libiberty/concat.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/libiberty/concat.c b/libiberty/concat.c
index 4cb1df3baf3..3a6b4ca71e8 100644
--- a/libiberty/concat.c
+++ b/libiberty/concat.c
@@ -169,6 +169,9 @@ loop:
str = reconcat (str, "pre-", str, NULL);
@end example
+Note: don't try to append string(s) to the a NULL string,
+as the process will stop at the first NULL argument.
+
@end deftypefn
*/
--
2.39.2
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] libiberty/reconcat: Add note about append string to NULL
2023-12-18 3:44 ` [PATCH 2/2] libiberty/reconcat: Add note about append string to NULL YunQiang Su
@ 2023-12-18 7:56 ` Jakub Jelinek
0 siblings, 0 replies; 5+ messages in thread
From: Jakub Jelinek @ 2023-12-18 7:56 UTC (permalink / raw)
To: YunQiang Su; +Cc: gcc-patches, ian, pinskia, matoro_mailinglist_gcc-patches
On Mon, Dec 18, 2023 at 11:44:22AM +0800, YunQiang Su wrote:
> For reconcat, if the `optr` can only be used as the last one
> of string list, aka, we cannot append something to it.
> Let's add some note into the document.
>
> libiberty:
> * concat.c (reconcat): Add note about append string to NULL
> into document.
> ---
> libiberty/concat.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/libiberty/concat.c b/libiberty/concat.c
> index 4cb1df3baf3..3a6b4ca71e8 100644
> --- a/libiberty/concat.c
> +++ b/libiberty/concat.c
> @@ -169,6 +169,9 @@ loop:
> str = reconcat (str, "pre-", str, NULL);
> @end example
>
> +Note: don't try to append string(s) to the a NULL string,
> +as the process will stop at the first NULL argument.
> +
> @end deftypefn
I think this is unnecessary and misleading.
The fact that NULL is the variable argument terminator is already clearly
documented, and first argument to reconcat can be NULL just fine,
so one just needs to be careful.
Jakub
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] MIPS: host_detect_local_cpu, init ret with concat [PR112759]
2023-12-18 3:44 [PATCH 1/2] MIPS: host_detect_local_cpu, init ret with concat [PR112759] YunQiang Su
2023-12-18 3:44 ` [PATCH 2/2] libiberty/reconcat: Add note about append string to NULL YunQiang Su
@ 2023-12-18 8:10 ` Jakub Jelinek
2023-12-18 22:59 ` YunQiang Su
1 sibling, 1 reply; 5+ messages in thread
From: Jakub Jelinek @ 2023-12-18 8:10 UTC (permalink / raw)
To: YunQiang Su; +Cc: gcc-patches, ian, pinskia, matoro_mailinglist_gcc-patches
On Mon, Dec 18, 2023 at 11:44:21AM +0800, YunQiang Su wrote:
> The function `reconcat` cannot append string(s) to NULL,
> as the concat process will stop at the first NULL.
>
> Let's initialize `ret` with `concat (" ", NULL)`, then
> it can be used by reconcat.
>
> gcc/
>
> PR target/112759
> * config/mips/driver-native.cc (host_detect_local_cpu):
> initialize ret with concat, so that it can be used by
> reconcat later.
> ---
> gcc/config/mips/driver-native.cc | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/gcc/config/mips/driver-native.cc b/gcc/config/mips/driver-native.cc
> index afc276f5278..471d1925eff 100644
> --- a/gcc/config/mips/driver-native.cc
> +++ b/gcc/config/mips/driver-native.cc
> @@ -44,7 +44,7 @@ const char *
> host_detect_local_cpu (int argc, const char **argv)
> {
> const char *cpu = NULL;
> - char *ret = NULL;
> + char *ret = concat(" ", NULL);
> char buf[128];
> FILE *f;
> bool arch;
> --
> 2.39.2
The formatting is wrong (no space after concat), but more importantly,
I don't think you want to return " " rather than NULL if there isn't
anything known and it is unnecessary to prefix everything with a space
when
gnu-user.h: " %{march=native:%<march=native %:local_cpu_detect(arch)}" \
gnu-user.h: " %{mtune=native:%<mtune=native %:local_cpu_detect(tune)}"
space is already present. Given the simple thing the function does,
I think much better would be to avoid any reconcat calls and just do one
concat, i.e.
--- gcc/config/mips/driver-native.cc 2023-01-02 09:32:58.422764590 +0100
+++ gcc/config/mips/driver-native.cc 2023-12-18 09:08:39.547609739 +0100
@@ -44,6 +44,7 @@ const char *
host_detect_local_cpu (int argc, const char **argv)
{
const char *cpu = NULL;
+ const char *nan2008 = "";
char *ret = NULL;
char buf[128];
FILE *f;
@@ -90,7 +91,7 @@ host_detect_local_cpu (int argc, const c
fallback_cpu:
#if defined (__mips_nan2008)
- ret = reconcat (ret, " -mnan=2008 ", NULL);
+ nan2008 = "-mnan=2008 ";
#endif
#ifdef HAVE_GETAUXVAL
@@ -104,7 +105,9 @@ fallback_cpu:
#endif
if (cpu)
- ret = reconcat (ret, ret, "-m", argv[0], "=", cpu, NULL);
+ ret = concat (nan2008, "-m", argv[0], "=", cpu, NULL);
+ else if (nan2008)
+ ret = concat (nan2008, NULL);
return ret;
}
Or, if you really want to use reconcat, drop that space from
before -mnan=2008 and use
ret = reconcat (ret, ret ? ret : "", "-m", argv[0], "=", cpu, NULL);
Jakub
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] MIPS: host_detect_local_cpu, init ret with concat [PR112759]
2023-12-18 8:10 ` [PATCH 1/2] MIPS: host_detect_local_cpu, init ret with concat [PR112759] Jakub Jelinek
@ 2023-12-18 22:59 ` YunQiang Su
0 siblings, 0 replies; 5+ messages in thread
From: YunQiang Su @ 2023-12-18 22:59 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: gcc-patches, ian, pinskia, matoro_mailinglist_gcc-patches
Jakub Jelinek <jakub@redhat.com> 于2023年12月18日周一 16:10写道:
>
> On Mon, Dec 18, 2023 at 11:44:21AM +0800, YunQiang Su wrote:
> > The function `reconcat` cannot append string(s) to NULL,
> > as the concat process will stop at the first NULL.
> >
> > Let's initialize `ret` with `concat (" ", NULL)`, then
> > it can be used by reconcat.
> >
> > gcc/
> >
> > PR target/112759
> > * config/mips/driver-native.cc (host_detect_local_cpu):
> > initialize ret with concat, so that it can be used by
> > reconcat later.
> > ---
> > gcc/config/mips/driver-native.cc | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/gcc/config/mips/driver-native.cc b/gcc/config/mips/driver-native.cc
> > index afc276f5278..471d1925eff 100644
> > --- a/gcc/config/mips/driver-native.cc
> > +++ b/gcc/config/mips/driver-native.cc
> > @@ -44,7 +44,7 @@ const char *
> > host_detect_local_cpu (int argc, const char **argv)
> > {
> > const char *cpu = NULL;
> > - char *ret = NULL;
> > + char *ret = concat(" ", NULL);
> > char buf[128];
> > FILE *f;
> > bool arch;
> > --
> > 2.39.2
>
> The formatting is wrong (no space after concat), but more importantly,
I will fix the format.
> I don't think you want to return " " rather than NULL if there isn't
> anything known and it is unnecessary to prefix everything with a space
> when
> gnu-user.h: " %{march=native:%<march=native %:local_cpu_detect(arch)}" \
> gnu-user.h: " %{mtune=native:%<mtune=native %:local_cpu_detect(tune)}"
> space is already present. Given the simple thing the function does,
> I think much better would be to avoid any reconcat calls and just do one
> concat, i.e.
> --- gcc/config/mips/driver-native.cc 2023-01-02 09:32:58.422764590 +0100
> +++ gcc/config/mips/driver-native.cc 2023-12-18 09:08:39.547609739 +0100
> @@ -44,6 +44,7 @@ const char *
> host_detect_local_cpu (int argc, const char **argv)
> {
> const char *cpu = NULL;
> + const char *nan2008 = "";
> char *ret = NULL;
> char buf[128];
> FILE *f;
> @@ -90,7 +91,7 @@ host_detect_local_cpu (int argc, const c
>
> fallback_cpu:
> #if defined (__mips_nan2008)
> - ret = reconcat (ret, " -mnan=2008 ", NULL);
> + nan2008 = "-mnan=2008 ";
> #endif
>
> #ifdef HAVE_GETAUXVAL
> @@ -104,7 +105,9 @@ fallback_cpu:
> #endif
>
> if (cpu)
> - ret = reconcat (ret, ret, "-m", argv[0], "=", cpu, NULL);
> + ret = concat (nan2008, "-m", argv[0], "=", cpu, NULL);
> + else if (nan2008)
> + ret = concat (nan2008, NULL);
>
> return ret;
> }
>
>
> Or, if you really want to use reconcat, drop that space from
> before -mnan=2008 and use
> ret = reconcat (ret, ret ? ret : "", "-m", argv[0], "=", cpu, NULL);
>
Good idea.
Maybe that we can use
ret = reconcat (ret, "-m", argv[0], "=", cpu, ret, NULL);
Then won't worry about ret is NULL here.
One reason I want to use reconcat is that I try to avoid something
wrong in future,
if somebody else add some new detect conditions.
With reconcat, the person will need only copy/paste ;)
It will reduce the possibility for doing something wrong in future.
And let's add a comment here.
> Jakub
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-12-18 22:59 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-12-18 3:44 [PATCH 1/2] MIPS: host_detect_local_cpu, init ret with concat [PR112759] YunQiang Su
2023-12-18 3:44 ` [PATCH 2/2] libiberty/reconcat: Add note about append string to NULL YunQiang Su
2023-12-18 7:56 ` Jakub Jelinek
2023-12-18 8:10 ` [PATCH 1/2] MIPS: host_detect_local_cpu, init ret with concat [PR112759] Jakub Jelinek
2023-12-18 22:59 ` YunQiang Su
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).