Re: Remove _fstati64 and _stati64 defs for mingw32

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: Remove _fstati64 and _stati64 defs for mingw32

Jan Stary
> These are not used anywhere in sox

What makes you say that? util.h says

#ifdef _stati64
#define stat _stati64
#else
#define stat _stat
#endif

So on a system which does have _stati64,
that is used as 'stat'.

(Not that I know a system with _stati64.)

> and already defined in mingw32 4.2.1.

Well, isn't 'stat' defined on every reasonable system?
Isn't the purpose of the above to redefine it?

Which might be strange: a system with _stati64
is probably already using it as its own stat, right?

I don't know why sox doesn't just use the given
system's stat everywhere.



------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and
threat landscape has changed and how IT managers can respond. Discussions
will include endpoint security, mobile security and the latest in malware
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
Sox-users mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/sox-users
Reply | Threaded
Open this post in threaded view
|

Re: Remove _fstati64 and _stati64 defs for mingw32

Motiejus Jakštys
On Tue, Sep 25, 2012 at 09:13:57AM +0200, Jan Stary wrote:
> > These are not used anywhere in sox
>
> What makes you say that? util.h says

Late night I guess. :)

> #ifdef _stati64
> #define stat _stati64
> #else
> #define stat _stat
> #endif
>
> So on a system which does have _stati64,
> that is used as 'stat'.

i586-mingw32msvc/include/sys/stat.h:

struct _stati64 {
    _dev_t st_dev;
    ...
};

That way if "util.h" is included before <sys/stat.h>, the latter is
translated to

struct stat { /* replaced by macro defined in "util.h" /*
    _dev_t st_dev;
    ...
};


> (Not that I know a system with _stati64.)

mingw32 4.2.1.dfsg-2ubuntu1

> Well, isn't 'stat' defined on every reasonable system?
> Isn't the purpose of the above to redefine it?

It is. But it is redefining it incorrectly (_stati64 is not a macro,
it's a struct!).

> Which might be strange: a system with _stati64
> is probably already using it as its own stat, right?

I guess so. This is really clumsy.

> I don't know why sox doesn't just use the given
> system's stat everywhere.

"util.h" is really strange. Maybe somebody knows why:

1. sox checking for _stati64 constant?
2. sox redefining stat?

I was trying to solve the issue doing least impact possible, wrongly. It
would be nice if somebody could explain what was the intent behind these
stat redefinitions.

Regards,
Motiejus

------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and
threat landscape has changed and how IT managers can respond. Discussions
will include endpoint security, mobile security and the latest in malware
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
Sox-users mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/sox-users

signature.asc (853 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Remove _fstati64 and _stati64 defs for mingw32

Jan Stary
On Sep 25 08:50:08, Motiejus Jakštys wrote:

> On Tue, Sep 25, 2012 at 09:13:57AM +0200, Jan Stary wrote:
> > > These are not used anywhere in sox
> >
> > What makes you say that? util.h says
>
> Late night I guess. :)
>
> > #ifdef _stati64
> > #define stat _stati64
> > #else
> > #define stat _stat
> > #endif
> >
> > So on a system which does have _stati64,
> > that is used as 'stat'.
>
> i586-mingw32msvc/include/sys/stat.h:
>
> struct _stati64 {
>     _dev_t st_dev;
>     ...
> };
>
> That way if "util.h" is included before <sys/stat.h>,

you mean util.h being included _after_ sys/stat.h;
which it typically is.

> the latter is translated to
>
> struct stat { /* replaced by macro defined in "util.h" /*
>     _dev_t st_dev;
>     ...
> };

Yes. I thought that's the purpose of having those #define's.

> > (Not that I know a system with _stati64.)
>
> mingw32 4.2.1.dfsg-2ubuntu1
>
> > Well, isn't 'stat' defined on every reasonable system?
> > Isn't the purpose of the above to redefine it?
>
> It is. But it is redefining it incorrectly (_stati64 is not a macro,
> it's a struct!).

Of course it's a struct; 'stat' is a struct too.

> > Which might be strange: a system with _stati64
> > is probably already using it as its own stat, right?
>
> I guess so. This is really clumsy.
>
> > I don't know why sox doesn't just use the given
> > system's stat everywhere.
>
> "util.h" is really strange. Maybe somebody knows why:
>
> 1. sox checking for _stati64 constant?

No, not 'constant'. SoX checks if '_stati64' is defined
- which is probably a way to find out whether we are on
a system that has struct _stati64.

> 2. sox redefining stat?
>
> I was trying to solve the issue doing least impact possible, wrongly. It
> would be nice if somebody could explain what was the intent behind these
> stat redefinitions.

Agreed.

        Jan


------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and
threat landscape has changed and how IT managers can respond. Discussions
will include endpoint security, mobile security and the latest in malware
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
Sox-users mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/sox-users
Reply | Threaded
Open this post in threaded view
|

Re: Remove _fstati64 and _stati64 defs for mingw32

Motiejus Jakštys
On Tue, Sep 25, 2012 at 10:00:31AM +0200, Jan Stary wrote:

> On Sep 25 08:50:08, Motiejus Jakštys wrote:
> > On Tue, Sep 25, 2012 at 09:13:57AM +0200, Jan Stary wrote:
> > > > These are not used anywhere in sox
> > >
> > > What makes you say that? util.h says
> >
> > i586-mingw32msvc/include/sys/stat.h:
> >
> > struct _stati64 {
> >     _dev_t st_dev;
> >     ...
> > };
> >
> > That way if "util.h" is included before <sys/stat.h>,
>
> you mean util.h being included _after_ sys/stat.h;
> which it typically is.
formats_i.c:21: #include "sox_i.h"
formats_i.c:23: #include <sys/stat.h>

sox_i.h:27: #include "util.h"

So as far as I see it includes "util.h" _before_ <sys/stat.h>. At least
in formats_i.c.

> > the latter is translated to
> >
> > struct stat { /* replaced by macro defined in "util.h" /*
> >     _dev_t st_dev;
> >     ...
> > };
>
> Yes. I thought that's the purpose of having those #define's.

I thought that the purpose is to redefine "stat" so that when "struct
stat" variable is defined, it is automagically replaced to _stat or
_stati64, depending on the architecture. For example, in
formats_i.c:143:

    struct stat st;

Seems like we are talking about different use cases.

> > > (Not that I know a system with _stati64.)
> >
> > mingw32 4.2.1.dfsg-2ubuntu1
> >
> > > Well, isn't 'stat' defined on every reasonable system?
> > > Isn't the purpose of the above to redefine it?
> >
> > It is. But it is redefining it incorrectly (_stati64 is not a macro,
> > it's a struct!).
>
> Of course it's a struct; 'stat' is a struct too.
It depends on how one intends to use that struct. Please clarify above
if I understand it correctly. If I do, then _stati64 should be a macro
and only a macro. Otherwise we are *redefining* `stat` in a peculiar
way, which is definitely not what we want.

> > > Which might be strange: a system with _stati64
> > > is probably already using it as its own stat, right?
> >
> > I guess so. This is really clumsy.
> >
> > > I don't know why sox doesn't just use the given
> > > system's stat everywhere.
> >
> > "util.h" is really strange. Maybe somebody knows why:
> >
> > 1. sox checking for _stati64 constant?
>
> No, not 'constant'. SoX checks if '_stati64' is defined
> - which is probably a way to find out whether we are on
> a system that has struct _stati64.
Again, see my argument above.

Thanks,
Motiejus

------------------------------------------------------------------------------
Don't let slow site performance ruin your business. Deploy New Relic APM
Deploy New Relic app performance management and know exactly
what is happening inside your Ruby, Python, PHP, Java, and .NET app
Try New Relic at no cost today and get our sweet Data Nerd shirt too!
http://p.sf.net/sfu/newrelic-dev2dev
_______________________________________________
Sox-users mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/sox-users

signature.asc (853 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Remove _fstati64 and _stati64 defs for mingw32

Jan Stary
On Oct 02 15:48:14, [hidden email] wrote:

> On Tue, Sep 25, 2012 at 10:00:31AM +0200, Jan Stary wrote:
> > On Sep 25 08:50:08, Motiejus Jakštys wrote:
> > > On Tue, Sep 25, 2012 at 09:13:57AM +0200, Jan Stary wrote:
> > > > > These are not used anywhere in sox
> > > >
> > > > What makes you say that? util.h says
> > >
> > > i586-mingw32msvc/include/sys/stat.h:
> > >
> > > struct _stati64 {
> > >     _dev_t st_dev;
> > >     ...
> > > };
> > >
> > > That way if "util.h" is included before <sys/stat.h>,
> >
> > you mean util.h being included _after_ sys/stat.h;
> > which it typically is.
>
> formats_i.c:21: #include "sox_i.h"
> formats_i.c:23: #include <sys/stat.h>
>
> sox_i.h:27: #include "util.h"
>
> So as far as I see it includes "util.h" _before_ <sys/stat.h>. At least
> in formats_i.c.

Then again, "util.h" itself includes <sys/stat.h> at the top.

On my system, the 'stat' structure that gets used in the code
is the one that is defined in <sys/stat.h>, as verified by
'cc -E formats_i.c'; but I am on OpenBSD, which does not
have _stati64 anyway. Can you please run 'cc -E formats_i.c'
on your _stati64 system and see what actually ends up in the code?


> > > the latter is translated to
> > >
> > > struct stat { /* replaced by macro defined in "util.h" /*
> > >     _dev_t st_dev;
> > >     ...
> > > };
> >
> > Yes. I thought that's the purpose of having those #define's.
>
> I thought that the purpose is to redefine "stat" so that when "struct
> stat" variable is defined, it is automagically replaced to _stat or
> _stati64, depending on the architecture. For example, in
> formats_i.c:143:
>
>     struct stat st;
>
> Seems like we are talking about different use cases.

Perhaps I worded that wrongly; you are right,
this seems to be the point of having those #defines.

Can someone who actually knoes shed some light on this please?

> > > It is. But it is redefining it incorrectly (_stati64 is not a macro,
> > > it's a struct!).
> >
> > Of course it's a struct; 'stat' is a struct too.
>
> It depends on how one intends to use that struct. Please clarify above
> if I understand it correctly. If I do, then _stati64 should be a macro
> and only a macro. Otherwise we are *redefining* `stat` in a peculiar
> way, which is definitely not what we want.

The way I see it is that on _certain_ systems (those that have _stati64
defined), the following snippet of util.h takes effect

        #ifdef _stati64
        #define stat _stati64
        #else
        #define stat _stat
        #fi

(note that this is marked as 'portability stuff' in util.h)

Then whenever you use 'struct stat', the preprocessor translates it
to 'struct _stati64'.

Plese try the above 'cc -E' on a system that actually has _stati64
and let's see.

        Jan


------------------------------------------------------------------------------
Don't let slow site performance ruin your business. Deploy New Relic APM
Deploy New Relic app performance management and know exactly
what is happening inside your Ruby, Python, PHP, Java, and .NET app
Try New Relic at no cost today and get our sweet Data Nerd shirt too!
http://p.sf.net/sfu/newrelic-dev2dev
_______________________________________________
Sox-users mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/sox-users
Reply | Threaded
Open this post in threaded view
|

Re: Remove _fstati64 and _stati64 defs for mingw32

Motiejus Jakštys
On Tue, Oct 02, 2012 at 08:38:56PM +0200, Jan Stary wrote:
> On Oct 02 15:48:14, [hidden email] wrote:
> > So as far as I see it includes "util.h" _before_ <sys/stat.h>. At least
> > in formats_i.c.
>
> Then again, "util.h" itself includes <sys/stat.h> at the top.

Yep. Missed that twice.

> On my system, the 'stat' structure that gets used in the code
> is the one that is defined in <sys/stat.h>, as verified by
> 'cc -E formats_i.c'; but I am on OpenBSD, which does not
> have _stati64 anyway. Can you please run 'cc -E formats_i.c'
> on your _stati64 system and see what actually ends up in the code?

Here we go: we see _stati64 defined twice:
http://paste.ubuntu.com/1256655/

> Can someone who actually knoes shed some light on this please?
>
> The way I see it is that on _certain_ systems (those that have _stati64
> defined), the following snippet of util.h takes effect
>
> #ifdef _stati64
> #define stat _stati64
> #else
> #define stat _stat
> #fi
So I can find out in CMakeLists whether _stati64 is defined as a struct
(my case). Then leave these ifdefs only if _stati64 and _stat are not
defined. Makes sense?

Regards,
Motiejus

------------------------------------------------------------------------------
Don't let slow site performance ruin your business. Deploy New Relic APM
Deploy New Relic app performance management and know exactly
what is happening inside your Ruby, Python, PHP, Java, and .NET app
Try New Relic at no cost today and get our sweet Data Nerd shirt too!
http://p.sf.net/sfu/newrelic-dev2dev
_______________________________________________
Sox-users mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/sox-users

signature.asc (853 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Remove _fstati64 and _stati64 defs for mingw32

Jan Stary
On Oct 02 20:23:50, [hidden email] wrote:

> On Tue, Oct 02, 2012 at 08:38:56PM +0200, Jan Stary wrote:
> > On Oct 02 15:48:14, [hidden email] wrote:
> > > So as far as I see it includes "util.h" _before_ <sys/stat.h>. At least
> > > in formats_i.c.
> >
> > Then again, "util.h" itself includes <sys/stat.h> at the top.
>
> Yep. Missed that twice.
>
> > On my system, the 'stat' structure that gets used in the code
> > is the one that is defined in <sys/stat.h>, as verified by
> > 'cc -E formats_i.c'; but I am on OpenBSD, which does not
> > have _stati64 anyway. Can you please run 'cc -E formats_i.c'
> > on your _stati64 system and see what actually ends up in the code?
>
> Here we go: we see _stati64 defined twice:
> http://paste.ubuntu.com/1256655/

That's not really a problem. Point is (IMHO) that the actual sox code,
such as the lsx_filelength() function further in the file,
now uses _stati64 for its operations.  Which is what is intended,
on a _stati64 system, right?

> > The way I see it is that on _certain_ systems (those that have _stati64
> > defined), the following snippet of util.h takes effect
> >
> > #ifdef _stati64
> > #define stat _stati64
> > #else
> > #define stat _stat
> > #fi
>
> So I can find out in CMakeLists whether _stati64 is defined as a struct
> (my case).

I don't know about CMakeLists, but it should already be apparent
from your system's includes whether _stati64 is defined or not.

> Then leave these ifdefs only if _stati64 and _stat are not
> defined. Makes sense?

You have lost me.

It seems to me now that sox does the expected thing:
uses struct _stati64 (instead of struct _stat) on
systems that have it. What was the problem again?


------------------------------------------------------------------------------
Don't let slow site performance ruin your business. Deploy New Relic APM
Deploy New Relic app performance management and know exactly
what is happening inside your Ruby, Python, PHP, Java, and .NET app
Try New Relic at no cost today and get our sweet Data Nerd shirt too!
http://p.sf.net/sfu/newrelic-dev2dev
_______________________________________________
Sox-users mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/sox-users
Reply | Threaded
Open this post in threaded view
|

Re: Remove _fstati64 and _stati64 defs for mingw32

Ulrich Klauer-2
Finally got around to read this not-too-recent thread ...

Motiejus Jak?tys <[hidden email]>:

> Reason: it did not compile on mingw32 4.2.1:

> [  1%] Building C object src/CMakeFiles/libsox.dir/formats_i.obj
> In file included from  
> /home/motiejus/code/stuff/sox/src/formats_i.c:23:  
> /usr/lib/gcc/i586-mingw32msvc/4.2.1-sjlj/../../../../i586-mingw32msvc/include/sys/stat.h:122: error: redefinition of ?struct  
> _stati64?
> make[2]: *** [src/CMakeFiles/libsox.dir/formats_i.obj] Error 1
> make[1]: *** [src/CMakeFiles/libsox.dir/all] Error 2
> make: *** [all] Error 2

> The way I see it is that on _certain_ systems (those that have _stati64
> defined), the following snippet of util.h takes effect
> #ifdef _stati64
> #define stat _stati64
> #else
> #define stat _stat
> #fi

I found a hint on the mythtv-dev mailing list how this may cause the  
double definition of struct _stati64: When the #define is in effect,  
the preprocessor turns a definition of "struct stat" into a (second)  
definition of "struct _stati64". It certainly looks from  
http://paste.ubuntu.com/1256655/ like this is the case. However ...

Jan Stary <[hidden email]>:

> On Oct 02 15:48:14, [hidden email] wrote:
> > So as far as I see it includes "util.h" _before_ <sys/stat.h>. At  
> least > in formats_i.c.
> Then again, "util.h" itself includes <sys/stat.h> at the top.

... this is true, so I didn't understand at first either how this  
could cause the problem. But then I noticed that the include in util.h  
is conditional: It only includes sys/stat.h if HAVE_SYS_STAT_H is set.  
And it seems we don't check for sys/stat.h at all when using cmake,  
but only when using the GNU build system.

Motiejus, could you verify that this is indeed the problem? I've  
prepared a small patch, but you could also simply remove the #ifdef  
around the include in util.h.

Ulrich

------------------------------------------------------------------------------
Master Visual Studio, SharePoint, SQL, ASP.NET, C# 2012, HTML5, CSS,
MVC, Windows 8 Apps, JavaScript and much more. Keep your skills current
with LearnDevNow - 3,200 step-by-step video tutorials by Microsoft
MVPs and experts. ON SALE this month only -- learn more at:
http://p.sf.net/sfu/learnmore_123012
_______________________________________________
Sox-users mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/sox-users

cmake.diff (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Remove _fstati64 and _stati64 defs for mingw32

Motiejus Jakštys
On Sun, Jan 13, 2013 at 10:57 AM, Ulrich Klauer <[hidden email]> wrote:
>
>
> Motiejus, could you verify that this is indeed the problem? I've prepared a
> small patch, but you could also simply remove the #ifdef around the include
> in util.h.

Hi, Ulrich,
thanks for taking care of this. Yes, this patch indeed fixes the
problem and now it compiles fine.

--
Motiejus Jakštys

------------------------------------------------------------------------------
Master Visual Studio, SharePoint, SQL, ASP.NET, C# 2012, HTML5, CSS,
MVC, Windows 8 Apps, JavaScript and much more. Keep your skills current
with LearnDevNow - 3,200 step-by-step video tutorials by Microsoft
MVPs and experts. ON SALE this month only -- learn more at:
http://p.sf.net/sfu/learnmore_123012
_______________________________________________
Sox-users mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/sox-users