bugGNU gettext - Bugs: bug #38783, Invalid check for __fsetlocking

 
 

You are not allowed to post comments on this tracker with your current authentication level.

bug #38783: Invalid check for __fsetlocking

Submitter:  LCID Fire <lcid_fire>
Submitted:  Sat 20 Apr 2013 10:22:05 AM UTC
 
Category:  Build Severity:  3 - Normal
Item Group:  None Status:  Not a Bug
Privacy:  Public Assigned to:  ueno
Open/Closed:  Closed
* Mandatory Fields

Add a New Comment (Rich Markup)
   

Comment Type & Canned Response:
       No canned response available

 

( Jump to the original submission )

Mon 12 Dec 2016 01:20:19 AM UTC, comment #7: 

The patch you are referring to (https://github.com/abergmeier/emscripten-gettext/commit/a6bac8f7b5ef21dd999198dad6ad8f1e8498cef6) has an effect only on platforms on which the questions "does the __fsetlocking function exist in libc" and "is the __fsetlocking function or macro declared in <stdio_ext.h>" have a different answer.

The major platforms which have __fsetlocking are glibc and Solaris (7 or newer), and they declare it as a function (not macro) in <stdio_ext.h>. On these platforms, the two questions have the same answer, therefore your patch is a no-op.

On musl 0.9.1 and Haiku, __fsetlocking is declared in <stdio_ext.h>. I don't have data about whether the function is installed in libc on these platforms. If not, your patch introduces a regression.

> The problem is that when you use a non gnu libc (for cross compiling), stdio_ext.h is not present and ____fsetlocking checking did not produce an error.


It sounds like your cross-compiling setup uses custom include files in combination with the preinstalled GNU libc in /usr/lib. If so, that would be an invalid cross-compiling setup.

Bruno Haible <haible>
Project Administrator
Mon 22 Apr 2013 05:26:38 AM UTC, comment #6: 

Anyway, such a decl check in intl.m4 is not for a usual case.  There is a comment before gt_CHECK_DECL([feof_unlocked], ...):

  dnl Use the *_unlocked functions only if they are declared.
  dnl (because some of them were defined without being declared in Solaris
  dnl 2.5.1 but were removed in Solaris 2.6, whereas we want binaries built
  dnl on Solaris 2.5.1 to run on Solaris 2.6).

I don't oppose merging the patch if you have a good reason like that (with a comment mentioning the actual portability problem).  Otherwise, I'd rather simply use AC_CHECK_HEADER instead of gt_CHECK_DECL.

(FWIW, __fsetlocking is still available in Solaris 11, the above comment is not appropriate for __fsetlocking.)

Daiki Ueno <ueno>
Project Administrator
Sat 20 Apr 2013 07:34:14 PM UTC, comment #5: 

Well, then how come gettext compiles and runs without any problems on FreeBSD or Mac OS X, where stdio_ext.h does not exist?  I'm asking for the actual error, not your understanding.  What did you try and what did you get precisely?

Daiki Ueno <ueno>
Project Administrator
Sat 20 Apr 2013 02:17:23 PM UTC, comment #4: 

I now applied the commit to both master (https://github.com/abergmeier/emscripten-gettext/commit/a6bac8f7b5ef21dd999198dad6ad8f1e8498cef6) and 0.18.2 branch (https://github.com/abergmeier/emscripten-gettext/commit/f8dba31c634cbef083a70024f42b47554107bad8) on git://git.savannah.gnu.org/gettext.git without any conflict.

My version of gettext was 0.18.2. The problem is that when you use a non gnu libc (for cross compiling), stdio_ext.h is not present and ____fsetlocking checking did not produce an error.
Semantically it does not make sense either, because it has to check whether stdio_ext.h is present first and fail if it is not (that is what the patch does). Only when both checks succeed (stdio_ext.h and __fsetlocking) you can later make the correct assumptions.

LCID Fire <lcid_fire>
Sat 20 Apr 2013 12:41:43 PM UTC, comment #3: 

Of course I looked at the patch before posting the reply and noticed that the change against intl.m4 does not apply to the current git master.  Are you talking about the change in localealias.c?

Anyway, you should better provide:

  • the version of your gettext and your OS
  • the actual error message
Daiki Ueno <ueno>
Project Administrator
Sat 20 Apr 2013 11:34:37 AM UTC, comment #2: 

Did you even bother to look at the patch?
I am not saying it is not checking, I am saying the check is wrong.
Please see the patch!

LCID Fire <lcid_fire>
Sat 20 Apr 2013 10:51:12 AM UTC, comment #1: 

It already has the check since 0.16 (the latest release is 0.18.2.1):
http://git.savannah.gnu.org/cgit/gettext.git/tree/gettext-runtime/m4/intl.m4#n233

Daiki Ueno <ueno>
Project Administrator
Sat 20 Apr 2013 10:22:05 AM UTC, original submission:  

Since `__fsetlocking` is not defined in a standard header, it has to be checked with it's explicit header ('stdio_ext.h'), or it will fail on some platforms.

Following a patch that fixes that problem: https://github.com/abergmeier/emscripten-gettext/commit/785e0701bdf4cc723c8482e46cf4a141d023e47e

LCID Fire <lcid_fire>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by haible (Updated the item)
  • -email is unavailable- added by ueno (Posted a comment)
  • -email is unavailable- added by lcid_fire (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

     

     

     

    Follow 5 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2016-12-12 haible StatusWorks For Me Not a Bug
    2016-11-27 haible CategoryNone Build
        Assigned toNone ueno
    2013-04-20 ueno StatusNone Works For Me
        Open/ClosedOpen Closed

    Back to the top


    Powered by Savane 3.9