Hi! Attached patch fixes libxl build. libgen.h is needed for basename(). Signed-off-by: Christoph Egger <Christoph.Egger@amd.com> -- ---to satisfy European Law for business letters: Advanced Micro Devices GmbH Einsteinring 24, 85609 Dornach b. Muenchen Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen Registergericht Muenchen, HRB Nr. 43632 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Christoph Egger writes ("[Xen-devel] libxl: build
fix"):> Attached patch fixes libxl build.
> libgen.h is needed for basename().
Nacked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Did you test this change ?
Looking at the manpage I have here, and the xl code, the version of
basename() expected by the libxl cpuid code is the GNU one, not the
POSIX one, and they have different semantics. I think we will have to
open code an implementation of basename.
I see that there is a lot of const-correctness misssing; if filename
had been declared const char* as it should your compiler would have
spotted the problem.
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
On Monday 08 November 2010 17:38:18 Ian Jackson wrote:> Christoph Egger writes ("[Xen-devel] libxl: build fix"): > > Attached patch fixes libxl build. > > libgen.h is needed for basename(). > > Nacked-by: Ian Jackson <ian.jackson@eu.citrix.com> > > Did you test this change ?Sure. W/o that patch, compiling fails with "implicit declaration" on NetBSD.> Looking at the manpage I have here,How does it differ to mine [1] ?> and the xl code, the version of > basename() expected by the libxl cpuid code is the GNU one, > not the POSIX one, and they have different semantics. I think we will have > to open code an implementation of basename.Can you imagine to make Xen tools prefer POSIX over GNU in general, please?> I see that there is a lot of const-correctness misssing; if filename > had been declared const char* as it should your compiler would have > spotted the problem.The compiler is pretty quiet w/o -Wconst-char -Wwrite-strings in respect to const-correctness. [1] http://netbsd.gw.com/cgi-bin/man-cgi?basename+3+NetBSD-current Christoph -- ---to satisfy European Law for business letters: Advanced Micro Devices GmbH Einsteinring 24, 85609 Dornach b. Muenchen Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen Registergericht Muenchen, HRB Nr. 43632 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Christoph Egger writes ("Re: [Xen-devel] libxl: build
fix"):> On Monday 08 November 2010 17:38:18 Ian Jackson wrote:
> > Did you test this change ?
>
> Sure. W/o that patch, compiling fails with
> "implicit declaration" on NetBSD.
No, I mean: did you execute the new code ? Never mind.
> > Looking at the manpage I have here,
>
> How does it differ to mine [1] ?
> [1] http://netbsd.gw.com/cgi-bin/man-cgi?basename+3+NetBSD-current
Look at the code and you will see that it assumes that basename does
not modify its argument. I''m about to push a const-correctness fix
which will make this more obvious and cause your broken attempt at a
fix not to compile :-).
> > and the xl code, the version of
> > basename() expected by the libxl cpuid code is the GNU one,
> > not the POSIX one, and they have different semantics. I think we will
have
> > to open code an implementation of basename.
>
> Can you imagine to make Xen tools prefer POSIX over GNU in general, please?
I agree that the current code is a mistake. The POSIX basename is
irritating and we shouldn''t use it. Simply open coding a search for a
''/'' will be fine.
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
On Monday 08 November 2010 18:01:54 Ian Jackson wrote:> Look at the code and you will see that it assumes that basename does > not modify its argument. I''m about to push a const-correctness fix > which will make this more obvious and cause your broken attempt at a > fix not to compile :-).I rebased my source tree and yes, it does not compile.> > > and the xl code, the version of > > > basename() expected by the libxl cpuid code is the GNU one, > > > not the POSIX one, and they have different semantics. I think we will > > > have to open code an implementation of basename. > > > > Can you imagine to make Xen tools prefer POSIX over GNU in general, > > please? > > I agree that the current code is a mistake. The POSIX basename is > irritating and we shouldn''t use it. Simply open coding a search for a > ''/'' will be fine.Attached patch replaces basename with strrchr(3). Signed-off-by: Christoph Egger <Christoph.Egger@amd.com> -- ---to satisfy European Law for business letters: Advanced Micro Devices GmbH Einsteinring 24, 85609 Dornach b. Muenchen Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen Registergericht Muenchen, HRB Nr. 43632 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Christoph Egger writes ("Re: [Xen-devel] libxl: build
fix"):> Attached patch replaces basename with strrchr(3).
...> - name = basename(filename);
> + name = strrchr(filename, ''/'');
*ahem*
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
On Tuesday 09 November 2010 12:54:45 Ian Jackson wrote:> Christoph Egger writes ("Re: [Xen-devel] libxl: build fix"): > > Attached patch replaces basename with strrchr(3). > > ... > > > - name = basename(filename); > > + name = strrchr(filename, ''/'');Sorry, should be name = strrchr(filename, ''/'') + 1; Christoph -- ---to satisfy European Law for business letters: Advanced Micro Devices GmbH Einsteinring 24, 85609 Dornach b. Muenchen Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen Registergericht Muenchen, HRB Nr. 43632 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On 09/11/10 12:58, Christoph Egger wrote:> On Tuesday 09 November 2010 12:54:45 Ian Jackson wrote: >> Christoph Egger writes ("Re: [Xen-devel] libxl: build fix"): >>> Attached patch replaces basename with strrchr(3). >> ... >> >>> - name = basename(filename); >>> + name = strrchr(filename, ''/''); > Sorry, should be > > name = strrchr(filename, ''/'') + 1; >No, it should be name = strrchr(filename, ''/''); if (name) name++; else name = filename; In fact even that isn''t right because basename(3) discards (or at least ignores) trailing ''/'' characters. jch _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
John Haxby writes ("Re: [Xen-devel] libxl: build
fix"):> No, it should be
>
> name = strrchr(filename, ''/'');
> if (name)
> name++;
> else
> name = filename;
As you say.
> In fact even that isn''t right because basename(3) discards (or at
least
> ignores) trailing ''/'' characters.
In this particular case, there won''t be any, because if there were
then opening the cpupool config file would fail.
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel