Firefox Bug 784402, Pointer Lock must respect iframe sandbox flag

Recently I’ve worked on the Firefox Bug 784402 – Pointer Lock must respect iframe sandbox flag.

This is a quick overview of what had to be done on the bug.

Sandbox flags

First lets check what the sandbox attribute does:
A quote from the w3c spec

The sandbox attribute, when specified, enables a set of extra restrictions on any content hosted by the iframe. Its value must be an unordered set of unique space-separated tokens that are ASCII case-insensitive. The allowed values are allow-forms, allow-popups, allow-same-origin, allow-scripts, and allow-top-navigation. When the attribute is set, the content is treated as being from a unique origin, forms and scripts are disabled, links are prevented from targeting other browsing contexts, and plugins are secured. The allow-same-origin keyword allows the content to be treated as being from the same origin instead of forcing it into a unique origin, the allow-top-navigation keyword allows the content to navigate its top-level browsing context, and the allow-forms, allow-popups and allow-scripts keywords re-enable forms, popups, and scripts respectively.

With pointerlock landing on Firefox 15, it was decided that a new sandbox flag should be created to restrict the pointerlock usage on embedded scripts in a page, so for example: if you add an advertisement script on your page, you don’t want to give the permissions to the advertisement to lock the pointer to itself.
To manage that, the allow-pointer-lock sandbox was created.

An overview of how the sandbox flags work:
List of flags:

/**
 * This flag prevents content from navigating browsing contexts other than
 * the sandboxed browsing context itself (or browsing contexts further
 * nested inside it), and the top-level browsing context.
 */
const unsigned long SANDBOXED_NAVIGATION  = 0x1;

/**
 * This flag prevents content from navigating their top-level browsing
 * context.
 */
const unsigned long SANDBOXED_TOPLEVEL_NAVIGATION = 0x2;

/**
 * This flag prevents content from instantiating plugins, whether using the
 * embed element, the object element, the applet element, or through
 * navigation of a nested browsing context, unless those plugins can be
 * secured.
 */
const unsigned long SANDBOXED_PLUGINS = 0x4;

/**
 * This flag forces content into a unique origin, thus preventing it from
 * accessing other content from the same origin.
 * This flag also prevents script from reading from or writing to the
 * document.cookie IDL attribute, and blocks access to localStorage.
 */
const unsigned long SANDBOXED_ORIGIN = 0x8;

/**
 * This flag blocks form submission.
 */
const unsigned long SANDBOXED_FORMS = 0x10;

/**
 * This flag blocks script execution.
 */
const unsigned long SANDBOXED_SCRIPTS = 0x20;

/**
 * This flag blocks features that trigger automatically, such as
 * automatically playing a video or automatically focusing a form control.
 */
const unsigned long SANDBOXED_AUTOMATIC_FEATURES = 0x40;

/**
 * This flag blocks the document from acquiring pointerlock.
 */
const unsigned long SANDBOXED_POINTER_LOCK = 0x80;

Parsing the flags

So we have a 32 bit integer to store the sandbox flags.

Breaking down the integer we have 8 bytes
We can represent each byte in hexadecimal format:

So the number 0xFFFFFFFF has all the bits turned ON

Knowing that, we could use each bit of the integer to represent a flag.
We don’t care about the decimal value of that integer, since we are using it to store flags and not values.
So by saying 0x1, we are telling to turn the first bit of the first byte on, 0x2 turns the second bit of the first byte on
0x10 on the other hand tells to turn the first bit of the second byte on.
Remember that we are using hexadecimal notation.

So in the end, what’s happening is that each flag is turning a different bit on the integer

Later we’ll be able to check if that specific bit is ON or OFF and determine the status of the flag.

One thing to keep in mind is that if the iframe doesn’t have the sandbox attribute, then all the flags are turned OFF by default.

<i frame></i frame>

If the iframe has an empty sandbox attribute, then all the flags are ON by default

<i frame sandbox=""></i frame>

To turn the flags off, you can specify the feature you want to enable in the sandbox attribute:

<i frame sandbox="allow-pointer-lock allow-same-origin></i frame>

In the snippet above both the allow-pointer-lock and allow-same-origin flag would be turned OFF, all the other flags would be ON

This is the code that parses the sandbox flags:

/**
 * A helper function that parses a sandbox attribute (of an <iframe> or
 * a CSP directive) and converts it to the set of flags used internally.
 *
 * @param aAttribute    the value of the sandbox attribute
 * @return              the set of flags
 */
uint32_t
nsContentUtils::ParseSandboxAttributeToFlags(const nsAString& aSandboxAttrValue)
{
  // If there's a sandbox attribute at all (and there is if this is being
  // called), start off by setting all the restriction flags.
  uint32_t out = SANDBOXED_NAVIGATION |
                 SANDBOXED_TOPLEVEL_NAVIGATION |
                 SANDBOXED_PLUGINS |
                 SANDBOXED_ORIGIN |
                 SANDBOXED_FORMS |
                 SANDBOXED_SCRIPTS |
                 SANDBOXED_AUTOMATIC_FEATURES |
                 SANDBOXED_POINTER_LOCK;

  if (!aSandboxAttrValue.IsEmpty()) {
    // The separator optional flag is used because the HTML5 spec says any
    // whitespace is ok as a separator, which is what this does.
    HTMLSplitOnSpacesTokenizer tokenizer(aSandboxAttrValue, ' ',
      nsCharSeparatedTokenizerTemplate<nsContentUtils::IsHTMLWhitespace>::SEPARATOR_OPTIONAL);

    while (tokenizer.hasMoreTokens()) {
      nsDependentSubstring token = tokenizer.nextToken();
      if (token.LowerCaseEqualsLiteral("allow-same-origin")) {
        out &= ~SANDBOXED_ORIGIN;
      } else if (token.LowerCaseEqualsLiteral("allow-forms")) {
        out &= ~SANDBOXED_FORMS;
      } else if (token.LowerCaseEqualsLiteral("allow-scripts")) {
        // allow-scripts removes both SANDBOXED_SCRIPTS and
        // SANDBOXED_AUTOMATIC_FEATURES.
        out &= ~SANDBOXED_SCRIPTS;
        out &= ~SANDBOXED_AUTOMATIC_FEATURES;
      } else if (token.LowerCaseEqualsLiteral("allow-top-navigation")) {
        out &= ~SANDBOXED_TOPLEVEL_NAVIGATION;
      } else if (token.LowerCaseEqualsLiteral("allow-pointer-lock")) {
        out &= ~SANDBOXED_POINTER_LOCK;
      }
    }
  }

  return out;
}

First all the flags are turned ON.
Then it checks if the sandbox attribute has any values, if it does it splits them and compares against the possible flags.
Once it finds a match, it does a BIT NEGATION on the flag and a BIT AND with the integer that has all the other flags.
What happens is that the flag being parsed is turned OFF.

In the end the integer with the status of all the flags is returned.

Locking the pointer

Now lets take a look at the code that checks for the allow-pointer-lock flag when an element requests pointerlock

bool
nsDocument::ShouldLockPointer(Element* aElement)
{
  // Check if pointer lock pref is enabled
  if (!Preferences::GetBool("full-screen-api.pointer-lock.enabled")) {
    NS_WARNING("ShouldLockPointer(): Pointer Lock pref not enabled");
    return false;
  }

  if (aElement != GetFullScreenElement()) {
    NS_WARNING("ShouldLockPointer(): Element not in fullscreen");
    return false;
  }

  if (!aElement->IsInDoc()) {
    NS_WARNING("ShouldLockPointer(): Element without Document");
    return false;
  }

  if (mSandboxFlags & SANDBOXED_POINTER_LOCK) {
    NS_WARNING("ShouldLockPointer(): Document is sandboxed and doesn't allow pointer-lock");
    return false;
  }

  // Check if the element is in a document with a docshell.
  nsCOMPtr ownerDoc = aElement->OwnerDoc();
  if (!ownerDoc) {
    return false;
  }
  if (!nsCOMPtr(ownerDoc->GetContainer())) {
    return false;
  }
  nsCOMPtr ownerWindow = ownerDoc->GetWindow();
  if (!ownerWindow) {
    return false;
  }
  nsCOMPtr ownerInnerWindow = ownerDoc->GetInnerWindow();
  if (!ownerInnerWindow) {
    return false;
  }
  if (ownerWindow->GetCurrentInnerWindow() != ownerInnerWindow) {
    return false;
  }

  return true;
}

The ShouldLockPointer method is called every time an element requests pointerlock, the method does some sanity checks and makes sure everything is correct.
To check for the allow-pointer-lock sandbox flag, a BIT AND with the mSandBoxFlags and the SANDBOX_POINTER_LOCK const is performed, we’ve looked at the SANDBOX_POINTER_LOCK flag before, it has the value of 0x80
So if pointerlock is allowed, the mSandboxFlags would have the SANDBOX_POINTER_LOCK flag OFF and the BIT AND would be false.

A big thanks to Ian Melven.
Ian is the one who implemented the sandbox attribute on Firefox and gave me some guidance on the PointerLock sandbox attribute bug.

Advertisements

DPS915 Workshop 1 – Initial Profile

Int the first workshop for the DPS915 course(Parallel Programming Fundamentals) we had to profile a simple application.
I wrote a previous blog post listing the steps to profile an application on osx.

The application we had to profile was:

// Profile a Serial Application - Workshop 1
 // w1.cpp

 #include <iostream>
 #include <iomanip>
 #include <cstdlib>
 #include <ctime>
 using namespace std;

 void init(float** a, int n) {
     float f = 1.0f / RAND_MAX;
     for (int i = 0; i < n; i++)
         for (int j = 0; j < n; j++)
             a[i][j] = rand() * f;
 }

 void add(float** a, float** b, float** c, int n) {
     for (int i = 0; i < n; i++)
         for (int j = 0; j < n; j++)
             c[i][j] = a[i][j] + 3.0f * b[i][j];
 }

 void multiply(float** a, float** b, float** c, int n) {
     for (int i = 0; i < n; i++)
         for (int j = 0; j < n; j++) {
             float sum = 0.0f;
             for (int k = 0; k < n; k++)
                 sum += a[i][k] * b[k][j];
             c[i][j] = sum;
         }
 }

 int main(int argc, char* argv[]) {
     // start timing
     time_t ts, te;
     ts = time(nullptr);

     // interpret command-line arguments
     if (argc != 3) {
         cerr << "**invalid number of arguments**" << endl;
         return 1;
     }
     int n  = atoi(argv[1]);   // size of matrices
     int nr = atoi(argv[2]);   // number of runs

     float** a = new float*[n];
     for (int i = 0; i < n; i++)
        a[i] = new float[n];
     float** b = new float*[n];
     for (int i = 0; i < n; i++)
        b[i] = new float[n];
     float** c = new float*[n];
     for (int i = 0; i < n; i++)
        c[i] = new float[n];
     srand(time(nullptr));
     init(a, n);
     init(b, n);

     for (int i = 0; i < nr; i++) {
         add(a, b, c, n);
         multiply(a, b, c, n);
     }

     for (int i = 0; i < n; i++)
        delete [] a[i];
     delete [] a;
     for (int i = 0; i < n; i++)
        delete [] b[i];
     delete [] b;
     for (int i = 0; i < n; i++)
        delete [] c[i];
     delete [] c;

     // elapsed time
     te = time(nullptr);
     cout << setprecision(0);
     cout << "Elapsed time : " << difftime(te, ts) << endl;
 }

We had to run the application with 12 different combinations to see how much time the program spent executing the “add” and “multiply” functions.

Here is the profile results:

To easy the process of generating the profile data, I create a bash script to automate the runs:

#!/bin/bash

# First Set
N[0]=80
NR[0]=50

N[1]=160
NR[1]=50

N[2]=320
NR[2]=50


# Second Set
N[3]=80
NR[3]=100

N[4]=160
NR[4]=100

N[5]=320
NR[5]=100


# Third Set
N[6]=80
NR[6]=200

N[7]=160
NR[7]=200

N[8]=320
NR[8]=200


# Fourth Set
N[9]=80
NR[9]=400

N[10]=160
NR[10]=400

N[11]=320
NR[11]=400


if [ $(uname) = "Darwin" ]
then
OS="mac"
  CC="g++-4.7"
else
OS="linux"
  CC="g++"
fi

echo "OS $OS"

OPTIONS="-std=c++0x -O2 -g -pg"
OBJ="w1"
SRC="w1.cpp"

INSTRUMENT_TEMPLATE="/Applications/Xcode.app/Contents/Applications/Instruments.app/Contents/Resources/templates/Time Profiler.tracetemplate"
#compile workshop
$CC $OPTIONS -o $OBJ $SRC

#generate profile info
for i in {0..11}
do
echo "Running ${i}th set"
  if [ $OS = "mac" ]
  then
echo "Running on MacOS"
    instruments -t "$INSTRUMENT_TEMPLATE" -D results/mac/"${N[$i]}x${NR[$i]}.log" $OBJ ${N[$i]} ${NR[$i]}
  else
echo "Running some linux distro."
    ./$OBJ ${N[$i]} ${NR[$i]}
    gprof -p $OBJ > "results/linux/${N[$i]}x${NR[$i]}.log"
  fi
done

The script works both on mac and linux.
If it’s running on a mac, it uses the Instruments Time Profiler, on a linux distro it uses gprof.

I’m committing all my course work to github

Any suggestions are more than welcome 🙂


Using Instruments Time Profiler

Gprof problem

On OSX 10.8.1 (Mountain Lion) the gnu profiling tool wasn’t working.
I’ve looked it up online and there was very little documentation about the problem.
I read in a couple of places saying that gprof in fact didn’t work but I couldn’t find any final answers.
Basically what happened is that when the program was compiled with the “pg” option, the gmon.out file was not created, thus not being able to run gprof to gather profile information for a specific program.

At first I thought the problem could be related to the fact that I was running gcc 4.2.1(the one that comes by default with XCode) so I tried to compile the latest version of gcc from source to check if it solved the problem.
I compiled gcc version 4.7.1. However it didn’t fix the problem.

I even try linking the profiling lib manually, but the gmon.out file was not being created.

**I’m still trying to find why the gmon.out file wasn’t being created, if anybody knows the reason or have any suggestions please leave a comment below.
My next step will be to compile the libc from source to add some profile symbols.
I’m following these references:

A couple of resources that are not related to gprof but nevertheless very useful:

 

Time Profiler

With all that being said, I needed to profile a c++ program on the mac, so I went looking for alternatives.

Luckly, I found that XCode comes with some extra tools called Instruments
A few tools included in the Instruments toolset are:

  • Allocations
  • Leaks
  • Activity Monitor
  • Time Profiler
  • System Trace
  • Automation
  • Energy Diagnotics

To get started with the Time Profiler is very simple, you first need to create a Xcode project.

Select the Profile option under Product (Command + I)

Select the Time Profiler template

Finally it will display the profile of your application

So far so good, I managed to generate profile information for my application. However, what if I wanted to get the information via the command line?
In my case I had to run the same application several times with different arguments to inspect how some functions behaved in certain situations and if they needed some optimizations.
With that in mind, running the time profiler via XCode was out of the question since I would need to manually modify the arguments and run the profiler each single time.
Instead I created a bash script to automate the runs.

Now I needed to find how to run the Instruments Time Profiler via the command line.
It wasn’t easy, there is very few documentation online and the manual has some outdated information.
Instead of [-d document] the correct is [-D document]
Anyway, to run Instruments from the command line:

instruments -t PathToTemplate -D ProfileResults YourApplication [list of arguments]

To see a list with all the available templates:

instruments -s

The result is a trace file that will contain the information regarding the profiling of the application.


Building Firefox on Mountain Lion 10.8

All the work that I’ve done on Firefox so far has been on a linux box.
I bought a mac recently so I’m in the process of switching all my dev tools.
To build Firefox on a mac is almost as straight forward as building on a linux distro.

Here are the steps:

1.

First you’ll need to install macports.
Download the pkg installer for Mountain Lion or whatever version you are running and install macports

After the installation you’ll need to restart your shell so the $PATH gets updated.
You can find more details here

Once macports is installed:

$ sudo port selfupdate
$ sudo port sync
$ sudo port install libidl autoconf213 yasm mercurial ccache

The commands above will install all the dependencies you need to build firefox.

**More info on how to configure ccache here

2.

Next it’s time to checkout the source code.

hg clone http://hg.mozilla.org/mozilla-central

It might take a while to clone the whole repo.

3.

Now that you have both the dev dependencies and the source code the last thing missing is a .mozconfig file.
Below is a default configuration:

ac_add_options --enable-debug
ac_add_options --enable-trace-malloc
ac_add_options --enable-accessibility
ac_add_options --enable-signmar

# Enable parallel compiling
mk_add_options MOZ_MAKE_FLAGS="-j12"

# Treat warnings as errors in directories with FAIL_ON_WARNINGS.
ac_add_options --enable-warnings-as-errors
ac_add_options --with-ccache

# Package js shell.
export MOZ_PACKAGE_JSSHELL=1

You can find more info about .mozconfig here

4.

Now it is time to start building.

First run:

make -f client.mk configure

That will make sure everything is setup properly, if you don’t see any error messages then you can start the build:

make -f client.mk build > build.out

A trick is to redirect the output of make to a file, it not only makes it easier to spot errors but it also decreases the build time.

Depending on your computer the build might take some time, don’t expect the build to finish before 15min, it will probably take something between 30min to 2h

5.

Once the build is done, you can run Firefox by going to dir obj-dir/dist/NightlyDebug.app/Contents/MacOS and launch the firefox executable.

References:
Simple Firefox build
Mac OS X Build Prerequisites


Getting started with CUDA on OSX 10.8 – Driver Problems

To install all the dev dependencies for CUDA enabled GPUs is not that bad, I faced a few issues but overall the documentation is pretty good.

You can find more information about how to get started here, it has all the links for the download of the driver + toolkit + SDK for windows, linux and mac

They also posted a PDF giving detail instructions about how to install everything.

 

Road Blocks

I’m running a MacBook Pro 2012 that comes with a GeForce GTM 650M.
On their website, they have the driver version 4.2 for download. However, I can update the CUDA driver to version 5.0.24 through the CUDA Preferences window under the System Preferences tab.

So after following the instructions they have posted on the Get Started pdf, I would get the message “Driver not supported” when running the deviceQuery test script.
I looked up online and found that this problem usually happened when the driver had a lower version than the SDK, I thought it was weird since I had downloaded all files they had instructed on the website.

I started browsing on the System Preferences when I saw the CUDA preferences tab.
On the tab it had the option to update the driver.
After the update, my driver was on version 5.0.24, and the deviceQuery test would work. \o/

After running the deviceQuery test, they suggested to run the bandwithTest to make sure the communication with the GPU was working properly.
To my surprise, when I ran the bandwithTest the computer crashed, some weird noises came from the case and a kernel panic messaged appeared.


Interval Since Last Panic Report:  75 sec
Panics Since Last Report:          2
Anonymous UUID:                    CD3F065C-4392-433E-8B7B-9D466743EE14

Tue Sep 11 23:16:23 2012
panic(cpu 4 caller 0xffffff802e8b7b95): Kernel trap at 0xffffff7faef9d18e, type 14=page fault, registers:
CR0: 0x0000000080010033, CR2: 0xffffff8191902000, CR3: 0x000000006b34b06c, CR4: 0x00000000001606e0
RAX: 0xffffff815123d000, RBX: 0x00000000406c5000, RCX: 0x00000000101b1400, RDX: 0xffffff8043302374
RSP: 0xffffff815117b650, RBP: 0xffffff815117b650, RSI: 0xffffff8043302004, RDI: 0xffffff80432ff804
R8:  0x00000000003f6a01, R9:  0xffffff815117b664, R10: 0x0000000000ffffff, R11: 0xffffff8100d10004
R12: 0xffffff80432ff804, R13: 0xffffff8043302374, R14: 0x0000000000000000, R15: 0xffffff8043302004
RFL: 0x0000000000010206, RIP: 0xffffff7faef9d18e, CS:  0x0000000000000008, SS:  0x0000000000000010
Fault CR2: 0xffffff8191902000, Error code: 0x0000000000000002, Fault CPU: 0x4

I wasn’t sure if the kernel panic was connected with the driver update, so I went back and ran some other scripts that come with the CUDA SDK, I ran the particles, simpleGL, volumeRender and a few others, then to my surprise again, when I ran the mergeSort another kernel panic was generated.
By now I was starting to get worried, I went back to the scripts dir and run a few others to make sure my GPU was still functioning properly, I ran the particles, simpleGL, volumeRender and the clock script, and again, after starting the clock script another kernel panic.

Now I knew for sure something was wrong, that shouldn’t be happening.

It was almost 12pm and I was getting tired and frustrated.
I did the only logical thing left to do… google it.

I entered the search: “mac 2012 crash with cuda driver 5”

 

Solution

To my relief it appeared that the kernel panics were in fact a known problem with the CUDA driver version 5 for the MacBook pro 2012.
I found this post on Adobe’s blog explaining the issue.
Apparently having the “Automatic Graphics Switching” option enable causes some CUDA applications to crash.
Turning the option off solves the problem.

Without the automatic graphics switching ON I ran the bandwithTest, mergeSort and clock apps and they worked just fine.

That Adobe’s blog post was created on August 29, so I believe that a fix for this problem should be coming out very soon.
Only Mountain Lion (Mac OSX v10.8) and Lion (Mac OSX v10.7) are affected by this bug.


Semester Roadmap

A new semester just began this week, and right now it looks to be one of the most challenging/exciting so far.

Bellow is my course list:

DPS915 Introduction to Parallel Programming
BTN710 Information Security
BTB720 Marketing Principles and Practices
BTS730 Project Management Methodologies
BTH740 Human Factors in Computing
CPP700 Co-op Integration and Career Planning

(I’m enrolled on the Software Development Degree at Seneca College)

I’m really looking forward for the “Introduction to Parallel Programming” and “Information Security”
The plan is to blog about both courses.
I’ll keep updating this post with the latest info about them.

On the DPS915 course, we will be learning how to run programs on the GPU using CUDA, by the end of the semester we’ll need to demonstrate the optimizations an application can have by porting some of the most heavy calculation areas to run on the GPU instead of the CPU.
My friend Simon(who’s also taking the course) has been working on a project to create md5 hashes for all possible combinations of words. Before, he faced some problems to generate the hashes, since the more chars in the words, the amount of time that it took to generate them was starting to become unfeasible.
The datastorage for all the hashes was also proving to be a problem.
The idea is to utilize the power of the GPU and leverage the amount of time it takes to generate the hashes.
For the data storage we are considering using redis, but we are still looking for other possibilities.
We’ll try to document the whole process of the project here.

A couple of other topics I’m also going to be blogging about is Blackberry10 Development and NodeJS+MongoDB.
Right now we are about to release the second iteration of Sobol, a construction management tool, and getting ready for iteration 3.
We have a lot of cool ideas to implement on the project and I want to document them here.

The new line of Blackberry devices will be coming out early next year, and RIM has been giving great support and incentive to developers. They redesigned all their APIs and the fact that the new BB10 platform is built on top of QNX makes even easier to port other developer tools.
The technologies supported are WebWorks, Cascades, Native and Java
WebWorks is the Javascript+Html5+Css stack
Cascades is the UI framework built on top of Qt
Native is C/C++
And since it’s possible to run Android apps on BB10 Java apps are also supported.

We’ll be focusing on Cascades and WebWorks development, and we already have a few apps on the oven that we plan to release in the coming weeks.

Finally, I also want to keep contributing to the Mozilla community, and will keep working + blogging about Firefox bugs.

Buckle up because this will be a hell of a semester!


Trimming spaces and comments in C

This is a simple C function that reads a line from a file and trim all comments and spaces.
You can see that the function receives a pointer to a file, a pointer to a char and an integer holding the size of the line to be read from the file.
The function then returns a line from the file without spaces(leading/trailing) and comments

Function signature:

char * readLine (FILE* fp, char* line, int size)

One thing I want to point out is the fact that it is possible to trim the string without creating a new one.
By using pointer arithimetics you can manipulate the chars of the line and remove anything you want.

For example, lets say you have a string like this:

***I’m using 0x00n just as an example to demonstrate the memory location of each char.

So the string A1 has two leading and trailling spaces.
Assuming that the line read from the file is: ” a1 ”

1.

First we remove the comments, none is this case:

s = strchr(line, '#');
if (s) *s = '\0';

We use strchr to search the string for any occurrences of #

If # is found in the string we set the null byte to the position of the first #

2.

The next step is to remove all the trailing spaces:

s = line + strlen(line) - 1;
while (isspace(*s)) s--;
*(s+1) = '\0';
  • First we assign to s the position of the last char in the line string.
  • Second we check if the char is a space using the isspace function(it checks not only for spaces, but for other delimiters as well). If the char is a space we subtract one from the s, meaning we subtract a char from s, setting s to point to one char before.
  • Once we find a char that’s not a space we break the loop.
  • Finally, we add one to s, and set the null byte to the position of the first space after the string ends.

3.

To remove leading spaces is even simpler:

s = line;
while (isspace(*s)) s++;

We set s to point to the first char in the string read from the file.
Then we loop through the string checking if the char is a space and incrementing the pointer by 1.

After all the trimming, s will point to the first non space char in the string, and the null byte will be positioned right after the last non space char. It will also have all the comments removed.

One thing to notice with this approach, is that the function must receive a char* and return a char*.
The reason beginning is that the char* needs to be declared in the function that is calling readLine, in this example the main. Since if not, the scope of the char* would be tight to the readLine function and thus the calling function would not be able to access the trimmed string.

Another possibility could be to manipulate the char* line itself, removing the necessity to return a new char.

This trim function could be adapt to not only work with files, but with different data structures as well.

If you have any suggestions or tips please leave a comment bellow 😀

Full Source Code:

#include
#include
#include

char *
readLine (FILE* fp, char* line, int size)
{
    char* s = NULL;

    while (!feof(fp) && fgets(line, size, fp)) {
        // Strip comments
        s = strchr(line, '#');
        if (s) *s = '\0';

        // Remove trailling spaces
        s = line + strlen(line) - 1;
        while (isspace(*s)) s--;
        *(s+1) = '\0';

        // Remove leading spaces
        s = line;
        while (isspace(*s)) s++;

        // Don't return empty lines
        if (*s) return s;
        printf("empty line\n");
    }

    return NULL;
}

int
main (void)
{
        FILE* fp = NULL;
        char line[256];
        char* s = NULL;
        fp = fopen("file", "r");
        if (!fp) return 1;
        while ((s = readLine(fp, line, sizeof(line)))) {
                printf("s: %s. || line: %s.\n", s, line);
        }
        return 0;
}

Test file:

#Some comments
start
  a1
  a2
  a3 #other comment
end

start
  b1
  b2
  start
    c1#comment....
    c2
  end
end